* Re: [PATCH net-next 2/3] sfc: refactor debug-or-warnings printks
From: David Miller @ 2017-01-26 19:36 UTC (permalink / raw)
To: ecree; +Cc: linux-net-drivers, netdev
In-Reply-To: <15f3ecb9-1785-8688-93b9-51d400ee12bc@solarflare.com>
From: Edward Cree <ecree@solarflare.com>
Date: Thu, 26 Jan 2017 17:53:48 +0000
> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index 5927c20..c640955 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -51,6 +51,15 @@
> #define EFX_WARN_ON_PARANOID(x) do {} while (0)
> #endif
>
> +/* if @cond then downgrade to debug, else print at @level */
> +#define netif_cond_dbg(priv, type, netdev, cond, level, fmt, args...) \
> + do { \
> + if (cond) \
> + netif_dbg(priv, type, netdev, fmt, ##args); \
> + else \
> + netif_ ## level(priv, type, netdev, fmt, ##args); \
> + } while (0)
> +
> /**************************************************************************
> *
> * Efx data structures
>
Please do not define locally in a driver an interface that looks like a generic
one and might be useful to code outside of your driver.
Thanks.
^ permalink raw reply
* Re: [PATCHv2 perf/core 5/7] tools lib bpf: Add bpf_program__pin()
From: Arnaldo Carvalho de Melo @ 2017-01-26 19:32 UTC (permalink / raw)
To: Joe Stringer, ast; +Cc: Wangnan (F), Daniel Borkmann, LKML, netdev
In-Reply-To: <b93c8045-7408-fb8d-f8a1-c0d11bdbc791@huawei.com>
Em Wed, Jan 25, 2017 at 10:18:22AM +0800, Wangnan (F) escreveu:
> On 2017/1/25 9:16, Joe Stringer wrote:
> > On 24 January 2017 at 17:06, Wangnan (F) <wangnan0@huawei.com> wrote:
> > > On 2017/1/25 9:04, Wangnan (F) wrote:
> > > Is it possible to use directory tree instead?
> > > %s/object/mapname
> > > %s/object/prog/instance
> > I don't think objects have names, so let's assume an object with two
> > program instances named foo, and one map named bar.
> > A call of bpf_object__pin(obj, "/sys/fs/bpf/myobj") would mount with
> > the following files and directories:
> > /sys/fs/bpf/myobj/foo/1
> > /sys/fs/bpf/myobj/foo/2
> > /sys/fs/bpf/myobj/bar
> > Alternatively, if you want to control exactly where you want the
> > progs/maps to be pinned, you can call eg
> > bpf_program__pin_instance(prog, "/sys/fs/bpf/wherever", 0) and that
> > instance will be mounted to /sys/fs/bpf/wherever, or alternatively
> > bpf_program__pin(prog, "/sys/fs/bpf/foo"), and you will end up with
> > /sys/fs/bpf/foo/{0,1}.
> > This looks pretty reasonable to me.
> It looks good to me.
Ok, please continue from perf/core, Ingo merged the first patch of this
patchset today,
- Arnaldo
^ permalink raw reply
* Re: [PATCH 0/7] pull request for net-next: batman-adv 2017-01-26
From: David Miller @ 2017-01-26 19:31 UTC (permalink / raw)
To: sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
In-Reply-To: <20170126164404.16074-1-sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
From: Simon Wunderlich <sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
Date: Thu, 26 Jan 2017 17:43:57 +0100
> this is the updated version of yesterdays feature/cleanup pull request for
> batman-adv which should go into net-next. We've followed your suggestion
> regarding the NET_XMIT_CN handling and modified Gaos patch accordingly.
>
> The remaining patches are untouched.
>
> Please pull or let me know of any problem!
Pulled, thanks.
^ permalink raw reply
* Re: [PATCH 02/14] tcp: fix mark propagation with fwmark_reflect enabled
From: Eric Dumazet @ 2017-01-26 19:28 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, davem, netdev
In-Reply-To: <20170126191935.GA26591@salvia>
On Thu, 2017-01-26 at 20:19 +0100, Pablo Neira Ayuso wrote:
> Right. This is not percpu as in IPv4.
>
> I can send a follow up patch to get this in sync with the way we do it
> in IPv4, ie. add percpu socket.
>
> Fine with this approach? Thanks!
Not really.
percpu sockets are going to slow down network namespace creation /
deletion and increase memory foot print.
IPv6 is cleaner because it does not really have to use different
sockets.
Ultimately would would like to have the same for IPv4.
I would rather carry the mark either in an additional parameter,
or in the flow that is already passed as a parameter.
^ permalink raw reply
* Re: [PATCH net v2 3/3] gtp: fix cross netns recv on gtp socket
From: David Miller @ 2017-01-26 19:22 UTC (permalink / raw)
To: aschultz; +Cc: pablo, netdev, Lionel.Gauthier, openbsc, laforge
In-Reply-To: <20170126151134.2892-4-aschultz@tpip.net>
From: Andreas Schultz <aschultz@tpip.net>
Date: Thu, 26 Jan 2017 16:11:34 +0100
> The use of the passed through netlink src_net to check for a
> cross netns operation was wrong. Using the GTP socket and the
> GTP netdevice is always correct (even if the netdev has been
> moved to new netns after link creation).
>
> Remove the now obsolete net field from gtp_dev.
>
> Signed-off-by: Andreas Schultz <aschultz@tpip.net>
Please at least compile test your submissions:
drivers/net/gtp.c: In function ‘gtp_newlink’:
drivers/net/gtp.c:677:8: error: too many arguments to function ‘gtp_encap_enable’
err = gtp_encap_enable(dev, gtp, fd0, fd1, src_net);
^
drivers/net/gtp.c:659:12: note: declared here
static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
^
drivers/net/gtp.c: At top level:
drivers/net/gtp.c:822:12: error: conflicting types for ‘gtp_encap_enable’
static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
^
drivers/net/gtp.c:659:12: note: previous declaration of ‘gtp_encap_enable’ was here
static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
^
drivers/net/gtp.c:659:12: warning: ‘gtp_encap_enable’ used but never defined
drivers/net/gtp.c:822:12: warning: ‘gtp_encap_enable’ defined but not used [-Wunused-function]
static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
^
scripts/Makefile.build:299: recipe for target 'drivers/net/gtp.o' failed
^ permalink raw reply
* Re: [BUG/RFC] vhost: net: big endian viring access despite virtio 1
From: Michael S. Tsirkin @ 2017-01-26 19:20 UTC (permalink / raw)
To: Halil Pasic
Cc: Jason Wang, kvm, Greg Kurz,
virtualization@lists.linux-foundation.org, netdev, Cornelia Huck
In-Reply-To: <a08f491a-1320-607e-f450-d32ce2c9e2f5@linux.vnet.ibm.com>
On Thu, Jan 26, 2017 at 06:39:14PM +0100, Halil Pasic wrote:
>
> Hi!
>
> Recently I have been investigating some strange migration problems on
> s390x.
>
> It turned out under certain circumstances vhost_net corrupts avail.idx by
> using wrong endianness.
>
> I managed to track the problem down (I'm pretty sure). It boils down to
> the following.
>
> When stopping vhost userspace (QEMU) calls vhost_net_set_backend with
> the fd argument set to -1, this leads to is_le being reset in
> vhost_vq_init_access. On a BE system resetting to legacy means resetting
> to BE. Usually this is not a problem, but in the case when oldubufs is not
> zero (which is not likely if no network stress applied) it is a problem.
> That code path needs to write avail.idx, and ends up using wrong
> endianness when doing that (but only on a BE system).
>
> That is the story in prose, now let's see the corresponding code annotated
> with some comments.
>
> from drivers/vhost/net.c:
> static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> {
> /* [..] some not too interesting stuff */
> sock = get_socket(fd);
> /* fd == -1 --> sock == NULL */
> if (IS_ERR(sock)) {
> r = PTR_ERR(sock);
> goto err_vq;
> }
>
> /* start polling new socket */
> oldsock = vq->private_data;
> if (sock != oldsock) {
> ubufs = vhost_net_ubuf_alloc(vq,
> sock && vhost_sock_zcopy(sock));
> if (IS_ERR(ubufs)) {
> r = PTR_ERR(ubufs);
> goto err_ubufs;
> }
>
> vhost_net_disable_vq(n, vq);
> ==> vq->private_data = sock;
> /* now vq->private_data is NULL */
> ==> r = vhost_vq_init_access(vq);
> if (r)
> goto err_used;
> /* vq endianness has been reset to BE on s390 */
> r = vhost_net_enable_vq(n, vq);
> if (r)
> goto err_used;
>
> ==> oldubufs = nvq->ubufs;
> /* here oldubufs might become != 0 */
> nvq->ubufs = ubufs;
>
> n->tx_packets = 0;
> n->tx_zcopy_err = 0;
> n->tx_flush = false;
> }
> mutex_unlock(&vq->mutex);
>
> if (oldubufs) {
> vhost_net_ubuf_put_wait_and_free(oldubufs);
> mutex_lock(&vq->mutex);
> ==> vhost_zerocopy_signal_used(n, vq);
> /* tries to update virtqueue structures; endianness is BE on s390
> * now (but should be LE for virtio-1) */
> mutex_unlock(&vq->mutex);
> }
> /*[..] rest of the function */
> }
>
> from drivers/vhost/vhost.c:
>
> int vhost_vq_init_access(struct vhost_virtqueue *vq)
> {
> __virtio16 last_used_idx;
> int r;
> bool is_le = vq->is_le;
>
> if (!vq->private_data) {
> ==> vhost_reset_is_le(vq);
> /* resets to native endianness and returns */
> return 0;
> }
>
> ==> vhost_init_is_le(vq);
> /* here we init is_le */
>
> r = vhost_update_used_flags(vq);
> if (r)
> goto err;
> vq->signalled_used_valid = false;
> if (!vq->iotlb &&
> !access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) {
> r = -EFAULT;
> goto err;
> }
> r = vhost_get_user(vq, last_used_idx, &vq->used->idx);
> if (r) {
> vq_err(vq, "Can't access used idx at %p\n",
> &vq->used->idx);
> goto err;
> }
> vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx);
> return 0;
>
> err:
> vq->is_le = is_le;
> return r;
> }
>
> AFAIU this can be fixed very simply by omitting the reset. Below the
> patch. I'm not sure though, the endianness handling ain't simple in
> vhost. Am I going in the right direction?
>
> We have a test (on s390x only) running for a couple of hours now and so
> far so good but it's a bit early to announce it is tested for s390x.
> If the patch is reasonable, I'm intend to do a version with proper
> attribution and stuff.
>
> By the way the reset was first introduced by
> https://lkml.org/lkml/2015/4/10/226 (dug it up in the hope that reasons
> for the reset were discussed -- but no enlightenment for me).
>
> Finally I would like to credit Dave Gilbert for hinting that the
> unreasonable avail.idx may be due to an endianness problem and Michael A.
> Tebolt for reporting the bug and testing.
>
> -------------------------8<--------------
> >From b26e2bbdc03832a0204ee2b42967a1b49a277dc8 Mon Sep 17 00:00:00 2001
> From: Halil Pasic <pasic@linux.vnet.ibm.com>
> Date: Thu, 26 Jan 2017 00:06:15 +0100
> Subject: [PATCH] vhost: remove useless/dangerous reset of is_le
>
> The reset of is_le does no good, but it contributes its fair share to a
> bug in vhost_net, which occurs if we have some oldubufs when stopping and
> setting a fd = -1 as a backend. Instead of doing something convoluted in
> vhost_net, let's just get rid of the reset.
>
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Fixes: commit 2751c9882b94
> ---
> drivers/vhost/vhost.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index d643260..08072a2 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1714,10 +1714,8 @@ int vhost_vq_init_access(struct vhost_virtqueue *vq)
> int r;
> bool is_le = vq->is_le;
>
> - if (!vq->private_data) {
> - vhost_reset_is_le(vq);
> + if (!vq->private_data)
> return 0;
> - }
>
> vhost_init_is_le(vq);
I think you do need to reset it, just maybe within vhost_init_is_le.
if (vhost_has_feature(vq, VIRTIO_F_VERSION_1))
vq->is_le = true;
else
vhost_reset_is_le(vq);
> --
> 2.8.4
^ permalink raw reply
* Re: [PATCH 02/14] tcp: fix mark propagation with fwmark_reflect enabled
From: Pablo Neira Ayuso @ 2017-01-26 19:19 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netfilter-devel, davem, netdev
In-Reply-To: <1485453760.5145.144.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Jan 26, 2017 at 10:02:40AM -0800, Eric Dumazet wrote:
> On Thu, 2017-01-26 at 17:37 +0100, Pablo Neira Ayuso wrote:
> > From: Pau Espin Pedrol <pespin.shar@gmail.com>
> >
> > Otherwise, RST packets generated by the TCP stack for non-existing
> > sockets always have mark 0.
> > The mark from the original packet is assigned to the netns_ipv4/6
> > socket used to send the response so that it can get copied into the
> > response skb when the socket sends it.
> >
> > Fixes: e110861f8609 ("net: add a sysctl to reflect the fwmark on replies")
> > Cc: Lorenzo Colitti <lorenzo@google.com>
> > Signed-off-by: Pau Espin Pedrol <pau.espin@tessares.net>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> > net/ipv4/ip_output.c | 1 +
> > net/ipv6/tcp_ipv6.c | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> > index fac275c48108..b67719f45953 100644
> > --- a/net/ipv4/ip_output.c
> > +++ b/net/ipv4/ip_output.c
> > @@ -1629,6 +1629,7 @@ void ip_send_unicast_reply(struct sock *sk, struct sk_buff *skb,
> > sk->sk_protocol = ip_hdr(skb)->protocol;
> > sk->sk_bound_dev_if = arg->bound_dev_if;
> > sk->sk_sndbuf = sysctl_wmem_default;
> > + sk->sk_mark = fl4.flowi4_mark;
> > err = ip_append_data(sk, &fl4, ip_reply_glue_bits, arg->iov->iov_base,
> > len, 0, &ipc, &rt, MSG_DONTWAIT);
> > if (unlikely(err)) {
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index 73bc8fc68acd..2b20622a5824 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -840,6 +840,7 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
> > dst = ip6_dst_lookup_flow(ctl_sk, &fl6, NULL);
> > if (!IS_ERR(dst)) {
> > skb_dst_set(buff, dst);
> > + ctl_sk->sk_mark = fl6.flowi6_mark;
> > ip6_xmit(ctl_sk, buff, &fl6, NULL, tclass);
> > TCP_INC_STATS(net, TCP_MIB_OUTSEGS);
> > if (rst)
>
>
> This patch is wrong.
>
> ctl_sk is a shared socket, and tcp_v6_send_response() can be called from
> many different cpus at the same time.
Right. This is not percpu as in IPv4.
I can send a follow up patch to get this in sync with the way we do it
in IPv4, ie. add percpu socket.
Fine with this approach? Thanks!
^ permalink raw reply
* Re: [PATCH] [net-next] ISDN: eicon: reduce stack size of sig_ind function
From: David Miller @ 2017-01-26 19:00 UTC (permalink / raw)
To: arnd; +Cc: mac, isdn, netdev, linux-kernel
In-Reply-To: <20170125221607.1102271-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 25 Jan 2017 23:15:53 +0100
> I noticed that this function uses a lot of kernel stack when the
> "latent entropy" plugin is enabled:
>
> drivers/isdn/hardware/eicon/message.c: In function 'sig_ind':
> drivers/isdn/hardware/eicon/message.c:6113:1: error: the frame size of 1168 bytes is larger than 1152 bytes [-Werror=frame-larger-than=]
>
> We currently don't warn about this, as we raise the warning limit
> to 2048 bytes in mainline, but I'd like to lower that limit again
> in the future, and this function can easily be changed to be more
> efficient and avoid that warning, by making some of its local
> variables 'const'.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied, thanks Arnd.
^ permalink raw reply
* Re: [PATCH net] bpf: expose netns inode to bpf programs
From: Andy Lutomirski @ 2017-01-26 19:07 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Linus Torvalds, Eric W. Biederman, David S . Miller,
Daniel Borkmann, David Ahern, Tejun Heo, Thomas Graf,
Network Development
In-Reply-To: <588A40D7.9070603@fb.com>
On Thu, Jan 26, 2017 at 10:32 AM, Alexei Starovoitov <ast@fb.com> wrote:
> On 1/26/17 10:12 AM, Andy Lutomirski wrote:
>>
>> On Thu, Jan 26, 2017 at 9:46 AM, Alexei Starovoitov <ast@fb.com> wrote:
>>>
>>> On 1/26/17 8:37 AM, Andy Lutomirski wrote:
>>>>>
>>>>>
>>>>> Think of bpf programs as safe kernel modules. They don't have
>>>>> confined boundaries and program authors, if not careful, can shoot
>>>>> themselves in the foot. We're not trying to prevent that because
>>>>> it's impossible to check that the program is sane. Just like
>>>>> it's impossible to check that kernel module is sane.
>>>>> But in case of bpf we check that bpf program is _safe_ from the kernel
>>>>> point of view. If it's doing some garbage, it's program's business.
>>>>> Does it make more sense now?
>>>>>
>>>>
>>>> With all due respect, I think this is not an acceptable way to think
>>>> about BPF at all. If you think of BPF this way, I think there needs
>>>> to be a real discussion at KS or similar as to whether this is okay.
>>>> The reason is simple: the kernel promises a stable ABI to userspace
>>>> but not to kernel modules. By thinking of BPF as more like a module,
>>>> you're taking a big shortcut that will either result in ABI breakage
>>>> down the road or in committing to a problematic stable ABI.
>>>
>>>
>>>
>>> you misunderstood the analogy.
>>> bpf abi is certainly stable. that's why we were careful of not
>>> exposing anything to it that is not already stable.
>>>
>>
>> In that case I don't understand what you're trying to say. Eric
>> thinks your patch exposes a bad interface. A bad interface for
>> userspace is a very different thing from a bad interface available to
>> kernel modules. Are you saying that BPF is kernel-module-like in that
>> the ABI exposed to BPF programs doesn't need to meet the same quality
>> standards as userspace ABIs?
>
>
> of course not.
> ns.inum is already exposed to user space as a value.
> This patch exposes it to bpf program in a convenient and stable way,
Here's what I'm imaging Eric is thinking:
ns.inum is currently exposed to userspace via procfs. In principle,
the value could be local to a namespace, though, which would enable
CRIU to be able to preserve namespace inode numbers across a
checkpoint+restore operation. If this happened, the contained and
restored procfs would see a different inode number than the outermost
procfs.
If you start exposing the raw ns.inum field to BPF programs and those
programs are not themselves scoped to a namespace, then this could
create a problem for CRIU.
But you told Eric that his nack doesn't matter, and maybe it would be
nice to ask him to clarify instead.
^ permalink raw reply
* Re: [PATCH net 1/5] ibmvnic: harden interrupt handler
From: Thomas Falcon @ 2017-01-26 19:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, wvoigt, linuxppc-dev, nfont, jallen
In-Reply-To: <20170126102802.4f920418@xeon-e3>
On 01/26/2017 12:28 PM, Stephen Hemminger wrote:
> On Wed, 25 Jan 2017 15:02:19 -0600
> Thomas Falcon <tlfalcon@linux.vnet.ibm.com> wrote:
>
>> static irqreturn_t ibmvnic_interrupt(int irq, void *instance)
>> {
>> struct ibmvnic_adapter *adapter = instance;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&adapter->crq.lock, flags);
>> + vio_disable_interrupts(adapter->vdev);
>> + tasklet_schedule(&adapter->tasklet);
>> + spin_unlock_irqrestore(&adapter->crq.lock, flags);
>> + return IRQ_HANDLED;
>> +}
>> +
> Why not use NAPI? rather than a tasklet
>
This interrupt function doesn't process packets, but message passing between firmware and driver for determining device capabilities and available resources, such as the number TX and RX queues.
^ permalink raw reply
* [PATCH] macvtap: Use kmalloc_array() in macvtap_queue_resize()
From: SF Markus Elfring @ 2017-01-26 19:00 UTC (permalink / raw)
To: netdev, David S. Miller, Francesco Ruggeri, Gao Feng, Jason Wang,
Marc Angel, Michael S. Tsirkin, Mike Rapoport, Willem de Bruijn,
Zhang Shengju
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 26 Jan 2017 19:47:38 +0100
A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/macvtap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 5c26653eceb5..1796d8f1ef47 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -1243,7 +1243,7 @@ static int macvtap_queue_resize(struct macvlan_dev *vlan)
int n = vlan->numqueues;
int ret, i = 0;
- arrays = kmalloc(sizeof *arrays * n, GFP_KERNEL);
+ arrays = kmalloc_array(n, sizeof(*arrays), GFP_KERNEL);
if (!arrays)
return -ENOMEM;
--
2.11.0
^ permalink raw reply related
* Re: NAPI on USB network drivers
From: Oliver Hartkopp @ 2017-01-26 18:59 UTC (permalink / raw)
To: Alexander Duyck
Cc: Hayes Wang, Oliver Neukum, netdev@vger.kernel.org,
linux-usb@vger.kernel.org
In-Reply-To: <CAKgT0UcxhhJVJZY-XrdD7ySkUHCnRy6GJ6_WWY8cVr1uE968CQ@mail.gmail.com>
On 01/25/2017 09:57 PM, Alexander Duyck wrote:
> On Wed, Jan 25, 2017 at 5:33 AM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>
> You could probably get around the o-o-o problem by enabling RPS for
> the interface. I have found that it works for me to do that in order
> to resolve o-o-o frames generated by VMs on virtual interfaces that
> can't use NAPI.
Sounds promising!
In fact I tried to fix this o-o-o stuff here:
http://marc.info/?l=linux-can&m=143637774606287&w=2
Do you have an example how to do it right?
Best regards,
Oliver
^ permalink raw reply
* Re: [PATCH net 1/5] ibmvnic: harden interrupt handler
From: Thomas Falcon @ 2017-01-26 18:57 UTC (permalink / raw)
To: David Miller; +Cc: netdev, wvoigt, linuxppc-dev, nfont, jallen
In-Reply-To: <20170126.125624.243097121899163842.davem@davemloft.net>
On 01/26/2017 11:56 AM, David Miller wrote:
> From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> Date: Thu, 26 Jan 2017 10:44:22 -0600
>
>> On 01/25/2017 10:04 PM, David Miller wrote:
>>> From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
>>> Date: Wed, 25 Jan 2017 15:02:19 -0600
>>>
>>>> Move most interrupt handler processing into a tasklet, and
>>>> introduce a delay after re-enabling interrupts to fix timing
>>>> issues encountered in hardware testing.
>>>>
>>>> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
>>> I don't think you have any idea what the real problem is. This looks
>>> like a hack, at best. Next patch you'll increase the delay to "20",
>>> right? And if that doesn't work you'll try "40".
>>>
>>> Or if you do know the reason, you need to explain it in detail in this
>>> commit message so that we can properly evaluate this patch.
>> You're right, I should have given more explanation in the commit message about the bug encountered and our reasons for this sort of fix. The issue is that there are some scenarios during the device init where multiple messages are sent by firmware in one interrupt request.
>>
>> We have observed behavior where messages are delayed, resulting in the interrupt handler completing before the delayed messages can be processed, fouling up the device bring-up in the device probing and elsewhere. The goal of the delay is to buy some time for the hypervisor to forward all the CRQ messages from the firmware.
> Then isn't there an event you can sleep and wait for, or a piece of state for
> you to poll and test for in a timeout based loop?
>
> This delay is a bad solution for the problem of waiting for A to happen
> before you do B.
>
Understood. We will come up with a better fix. Thanks for your attention.
^ permalink raw reply
* [PATCH net-next v2 2/4] net: dsa: Make most functions take a dsa_port argument
From: Florian Fainelli @ 2017-01-26 18:45 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170126184554.1325-1-f.fainelli@gmail.com>
In preparation for allowing platform data, and therefore no valid
device_node pointer, make most DSA functions takes a pointer to a
dsa_port structure whenever possible. While at it, introduce a
dsa_port_is_valid() helper function which checks whether port->dn is
NULL or not at the moment.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa.c | 15 ++++++++------
net/dsa/dsa2.c | 61 +++++++++++++++++++++++++++++-------------------------
net/dsa/dsa_priv.h | 4 ++--
3 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 1f3afeb673d6..07e863369e04 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -110,8 +110,9 @@ dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
/* basic switch operations **************************************************/
int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
- struct device_node *port_dn, int port)
+ struct dsa_port *dport, int port)
{
+ struct device_node *port_dn = dport->dn;
struct phy_device *phydev;
int ret, mode;
@@ -141,15 +142,15 @@ int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
static int dsa_cpu_dsa_setups(struct dsa_switch *ds, struct device *dev)
{
- struct device_node *port_dn;
+ struct dsa_port *dport;
int ret, port;
for (port = 0; port < DSA_MAX_PORTS; port++) {
if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
continue;
- port_dn = ds->ports[port].dn;
- ret = dsa_cpu_dsa_setup(ds, dev, port_dn, port);
+ dport = &ds->ports[port];
+ ret = dsa_cpu_dsa_setup(ds, dev, dport, port);
if (ret)
return ret;
}
@@ -364,8 +365,10 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
return ds;
}
-void dsa_cpu_dsa_destroy(struct device_node *port_dn)
+void dsa_cpu_dsa_destroy(struct dsa_port *port)
{
+ struct device_node *port_dn = port->dn;
+
if (of_phy_is_fixed_link(port_dn))
of_phy_deregister_fixed_link(port_dn);
}
@@ -389,7 +392,7 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
for (port = 0; port < DSA_MAX_PORTS; port++) {
if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
continue;
- dsa_cpu_dsa_destroy(ds->ports[port].dn);
+ dsa_cpu_dsa_destroy(&ds->ports[port]);
/* Clearing a bit which is not set does no harm */
ds->cpu_port_mask |= ~(1 << port);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 2cf489c5e90f..56c43ca7c049 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -78,14 +78,19 @@ static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
kref_put(&dst->refcount, dsa_free_dst);
}
-static bool dsa_port_is_dsa(struct device_node *port)
+static bool dsa_port_is_valid(struct dsa_port *port)
{
- return !!of_parse_phandle(port, "link", 0);
+ return !!port->dn;
}
-static bool dsa_port_is_cpu(struct device_node *port)
+static bool dsa_port_is_dsa(struct dsa_port *port)
{
- return !!of_parse_phandle(port, "ethernet", 0);
+ return !!of_parse_phandle(port->dn, "link", 0);
+}
+
+static bool dsa_port_is_cpu(struct dsa_port *port)
+{
+ return !!of_parse_phandle(port->dn, "ethernet", 0);
}
static bool dsa_ds_find_port(struct dsa_switch *ds,
@@ -119,7 +124,7 @@ static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
static int dsa_port_complete(struct dsa_switch_tree *dst,
struct dsa_switch *src_ds,
- struct device_node *port,
+ struct dsa_port *port,
u32 src_port)
{
struct device_node *link;
@@ -127,7 +132,7 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
struct dsa_switch *dst_ds;
for (index = 0;; index++) {
- link = of_parse_phandle(port, "link", index);
+ link = of_parse_phandle(port->dn, "link", index);
if (!link)
break;
@@ -150,13 +155,13 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
*/
static int dsa_ds_complete(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
int err;
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (!dsa_port_is_dsa(port))
@@ -196,7 +201,7 @@ static int dsa_dst_complete(struct dsa_switch_tree *dst)
return 0;
}
-static int dsa_dsa_port_apply(struct device_node *port, u32 index,
+static int dsa_dsa_port_apply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
int err;
@@ -211,13 +216,13 @@ static int dsa_dsa_port_apply(struct device_node *port, u32 index,
return 0;
}
-static void dsa_dsa_port_unapply(struct device_node *port, u32 index,
+static void dsa_dsa_port_unapply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
dsa_cpu_dsa_destroy(port);
}
-static int dsa_cpu_port_apply(struct device_node *port, u32 index,
+static int dsa_cpu_port_apply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
int err;
@@ -234,7 +239,7 @@ static int dsa_cpu_port_apply(struct device_node *port, u32 index,
return 0;
}
-static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
+static void dsa_cpu_port_unapply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
dsa_cpu_dsa_destroy(port);
@@ -242,13 +247,13 @@ static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
}
-static int dsa_user_port_apply(struct device_node *port, u32 index,
+static int dsa_user_port_apply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
const char *name;
int err;
- name = of_get_property(port, "label", NULL);
+ name = of_get_property(port->dn, "label", NULL);
if (!name)
name = "eth%d";
@@ -262,7 +267,7 @@ static int dsa_user_port_apply(struct device_node *port, u32 index,
return 0;
}
-static void dsa_user_port_unapply(struct device_node *port, u32 index,
+static void dsa_user_port_unapply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
if (ds->ports[index].netdev) {
@@ -274,7 +279,7 @@ static void dsa_user_port_unapply(struct device_node *port, u32 index,
static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
int err;
@@ -308,8 +313,8 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
}
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (dsa_port_is_dsa(port)) {
@@ -336,12 +341,12 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
static void dsa_ds_unapply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (dsa_port_is_dsa(port)) {
@@ -425,7 +430,7 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
dst->applied = false;
}
-static int dsa_cpu_parse(struct device_node *port, u32 index,
+static int dsa_cpu_parse(struct dsa_port *port, u32 index,
struct dsa_switch_tree *dst,
struct dsa_switch *ds)
{
@@ -433,7 +438,7 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
struct net_device *ethernet_dev;
struct device_node *ethernet;
- ethernet = of_parse_phandle(port, "ethernet", 0);
+ ethernet = of_parse_phandle(port->dn, "ethernet", 0);
if (!ethernet)
return -EINVAL;
@@ -466,13 +471,13 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
int err;
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (dsa_port_is_cpu(port)) {
@@ -533,7 +538,7 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
* to have access to a correct value, just like what
* net/dsa/dsa.c::dsa_switch_setup_one does.
*/
- if (!dsa_port_is_cpu(port))
+ if (!dsa_port_is_cpu(&ds->ports[reg]))
ds->enabled_port_mask |= 1 << reg;
}
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 63ae1484abae..16194a4bb2fe 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -50,8 +50,8 @@ struct dsa_slave_priv {
/* dsa.c */
int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
- struct device_node *port_dn, int port);
-void dsa_cpu_dsa_destroy(struct device_node *port_dn);
+ struct dsa_port *dport, int port);
+void dsa_cpu_dsa_destroy(struct dsa_port *dport);
const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
int dsa_cpu_port_ethtool_setup(struct dsa_switch *ds);
void dsa_cpu_port_ethtool_restore(struct dsa_switch *ds);
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 4/4] net: dsa: Move ports assignment closer to error checking
From: Florian Fainelli @ 2017-01-26 18:45 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170126184554.1325-1-f.fainelli@gmail.com>
Move the assignment of ports in _dsa_register_switch() closer to where
it is checked, no functional change. Re-order declarations to be
preserve the inverted christmas tree style.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa2.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 4c11619a818b..75f5d1f8554b 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -586,8 +586,8 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
{
struct device_node *np = dev->of_node;
- struct device_node *ports = dsa_get_ports(ds, np);
struct dsa_switch_tree *dst;
+ struct device_node *ports;
u32 tree, index;
int i, err;
@@ -595,6 +595,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
if (err)
return err;
+ ports = dsa_get_ports(ds, np);
if (IS_ERR(ports))
return PTR_ERR(ports);
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 1/4] net: dsa: Pass device pointer to dsa_register_switch
From: Florian Fainelli @ 2017-01-26 18:45 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170126184554.1325-1-f.fainelli@gmail.com>
In preparation for allowing dsa_register_switch() to be supplied with
device/platform data, pass down a struct device pointer instead of a
struct device_node.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 2 +-
drivers/net/dsa/mv88e6xxx/chip.c | 7 +++----
drivers/net/dsa/qca8k.c | 2 +-
include/net/dsa.h | 2 +-
net/dsa/dsa2.c | 7 ++++---
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 5cbb14f6a03b..bb210b12ad1b 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1894,7 +1894,7 @@ int b53_switch_register(struct b53_device *dev)
pr_info("found switch: %s, rev %i\n", dev->name, dev->core_rev);
- return dsa_register_switch(dev->ds, dev->ds->dev->of_node);
+ return dsa_register_switch(dev->ds, dev->ds->dev);
}
EXPORT_SYMBOL(b53_switch_register);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 5668e778ed1d..921e53351786 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4356,8 +4356,7 @@ static struct dsa_switch_driver mv88e6xxx_switch_drv = {
.ops = &mv88e6xxx_switch_ops,
};
-static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip,
- struct device_node *np)
+static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
{
struct device *dev = chip->dev;
struct dsa_switch *ds;
@@ -4372,7 +4371,7 @@ static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip,
dev_set_drvdata(dev, ds);
- return dsa_register_switch(ds, np);
+ return dsa_register_switch(ds, dev);
}
static void mv88e6xxx_unregister_switch(struct mv88e6xxx_chip *chip)
@@ -4456,7 +4455,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
if (err)
goto out_g2_irq;
- err = mv88e6xxx_register_switch(chip, np);
+ err = mv88e6xxx_register_switch(chip);
if (err)
goto out_mdio;
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 54d270d59eb0..c084aa484d2b 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -964,7 +964,7 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
mutex_init(&priv->reg_mutex);
dev_set_drvdata(&mdiodev->dev, priv);
- return dsa_register_switch(priv->ds, priv->ds->dev->of_node);
+ return dsa_register_switch(priv->ds, &mdiodev->dev);
}
static void
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 08b340403927..92fd795e9573 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -387,7 +387,7 @@ static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
}
void dsa_unregister_switch(struct dsa_switch *ds);
-int dsa_register_switch(struct dsa_switch *ds, struct device_node *np);
+int dsa_register_switch(struct dsa_switch *ds, struct device *dev);
#ifdef CONFIG_PM_SLEEP
int dsa_switch_suspend(struct dsa_switch *ds);
int dsa_switch_resume(struct dsa_switch *ds);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 866222a8f9bf..2cf489c5e90f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -578,8 +578,9 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
return ports;
}
-static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
{
+ struct device_node *np = dev->of_node;
struct device_node *ports = dsa_get_ports(ds, np);
struct dsa_switch_tree *dst;
u32 tree, index;
@@ -659,12 +660,12 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
return err;
}
-int dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+int dsa_register_switch(struct dsa_switch *ds, struct device *dev)
{
int err;
mutex_lock(&dsa2_mutex);
- err = _dsa_register_switch(ds, np);
+ err = _dsa_register_switch(ds, dev);
mutex_unlock(&dsa2_mutex);
return err;
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 0/4] net: dsa: Preparatory patches
From: Florian Fainelli @ 2017-01-26 18:45 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
Hi David,
This patch series extracts the 4 patches of the larger: net: dsa: Support for
pdata in dsa2 while we wait for feedback from Greg KH on the device references.
Changes in v2:
- rebased properly after the multi-MDIO bus support added to mv88e6xxx
Thanks!
Florian Fainelli (4):
net: dsa: Pass device pointer to dsa_register_switch
net: dsa: Make most functions take a dsa_port argument
net: dsa: Suffix function manipulating device_node with _dn
net: dsa: Move ports assignment closer to error checking
drivers/net/dsa/b53/b53_common.c | 2 +-
drivers/net/dsa/mv88e6xxx/chip.c | 11 ++---
drivers/net/dsa/qca8k.c | 2 +-
include/net/dsa.h | 2 +-
net/dsa/dsa.c | 15 ++++---
net/dsa/dsa2.c | 87 ++++++++++++++++++++++------------------
net/dsa/dsa_priv.h | 4 +-
7 files changed, 67 insertions(+), 56 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH net-next v2 3/4] net: dsa: Suffix function manipulating device_node with _dn
From: Florian Fainelli @ 2017-01-26 18:45 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170126184554.1325-1-f.fainelli@gmail.com>
Make it clear that these functions take a device_node structure pointer
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa2.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 56c43ca7c049..4c11619a818b 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -93,8 +93,8 @@ static bool dsa_port_is_cpu(struct dsa_port *port)
return !!of_parse_phandle(port->dn, "ethernet", 0);
}
-static bool dsa_ds_find_port(struct dsa_switch *ds,
- struct device_node *port)
+static bool dsa_ds_find_port_dn(struct dsa_switch *ds,
+ struct device_node *port)
{
u32 index;
@@ -104,8 +104,8 @@ static bool dsa_ds_find_port(struct dsa_switch *ds,
return false;
}
-static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
- struct device_node *port)
+static struct dsa_switch *dsa_dst_find_port_dn(struct dsa_switch_tree *dst,
+ struct device_node *port)
{
struct dsa_switch *ds;
u32 index;
@@ -115,7 +115,7 @@ static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
if (!ds)
continue;
- if (dsa_ds_find_port(ds, port))
+ if (dsa_ds_find_port_dn(ds, port))
return ds;
}
@@ -136,7 +136,7 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
if (!link)
break;
- dst_ds = dsa_dst_find_port(dst, link);
+ dst_ds = dsa_dst_find_port_dn(dst, link);
of_node_put(link);
if (!dst_ds)
@@ -545,7 +545,7 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
return 0;
}
-static int dsa_parse_member(struct device_node *np, u32 *tree, u32 *index)
+static int dsa_parse_member_dn(struct device_node *np, u32 *tree, u32 *index)
{
int err;
@@ -591,7 +591,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
u32 tree, index;
int i, err;
- err = dsa_parse_member(np, &tree, &index);
+ err = dsa_parse_member_dn(np, &tree, &index);
if (err)
return err;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v1] net: phy: micrel: add KSZ8795 ethernet switch
From: Andrew Lunn @ 2017-01-26 18:12 UTC (permalink / raw)
To: Sean Nyekjær; +Cc: netdev
In-Reply-To: <732c14ff-f0cf-67d5-bcf9-cbbf9a4391b3@prevas.dk>
> I remove the word "Switch" from the commit message and phy name.
> The rest is okay?
Yes.
> What is generally being done when to phy's have the same id, micrel
> have done this many times...
It should not be a problem. When you write a switch driver for the
switch, it will be in device tree. Look at the Marvell Switches as an
example. Switches don't probe based on the ID in registers 2 and 3,
because switches often respond to many different addresses, or don't
have an ID in register 2 and 3.
So it should not matter if the switch and the PHYs embedded in the
switch have the same ID.
When do you plan to post your switch driver?
Andrew
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: dsa: Pass device pointer to dsa_register_switch
From: Florian Fainelli @ 2017-01-26 18:37 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem
In-Reply-To: <20170126182808.13435-2-f.fainelli@gmail.com>
On 01/26/2017 10:28 AM, Florian Fainelli wrote:
> In preparation for allowing dsa_register_switch() to be supplied with
> device/platform data, pass down a struct device pointer instead of a
> struct device_node.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Need to revisit this:
drivers/net/dsa/mv88e6xxx/chip.c: In function 'mv88e6xxx_probe':
drivers/net/dsa/mv88e6xxx/chip.c:4460:3: error: implicit declaration of
function 'mv88e6xxx_mdio_unregister' [-Werror=implicit-function-declaration]
mv88e6xxx_mdio_unregister(chip);
^
cc1: some warnings being treated as errors
scripts/Makefile.build:293: recipe for target
'drivers/net/dsa/mv88e6xxx/chip.o' failed
make[6]: *** [drivers/net/dsa/mv88e6xxx/chip.o] Error 1
scripts/Makefile.build:551: recipe for target
'drivers/net/dsa/mv88e6xxx' failed
make[5]: *** [drivers/net/dsa/mv88e6xxx] Error 2
make[5]: *** Waiting for unfinished jobs....
Take that kbuild robot!
--
Florian
^ permalink raw reply
* [PATCH net-next 2/4] net: dsa: Make most functions take a dsa_port argument
From: Florian Fainelli @ 2017-01-26 18:28 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170126182808.13435-1-f.fainelli@gmail.com>
In preparation for allowing platform data, and therefore no valid
device_node pointer, make most DSA functions takes a pointer to a
dsa_port structure whenever possible. While at it, introduce a
dsa_port_is_valid() helper function which checks whether port->dn is
NULL or not at the moment.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa.c | 15 ++++++++------
net/dsa/dsa2.c | 61 +++++++++++++++++++++++++++++-------------------------
net/dsa/dsa_priv.h | 4 ++--
3 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 1f3afeb673d6..07e863369e04 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -110,8 +110,9 @@ dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
/* basic switch operations **************************************************/
int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
- struct device_node *port_dn, int port)
+ struct dsa_port *dport, int port)
{
+ struct device_node *port_dn = dport->dn;
struct phy_device *phydev;
int ret, mode;
@@ -141,15 +142,15 @@ int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
static int dsa_cpu_dsa_setups(struct dsa_switch *ds, struct device *dev)
{
- struct device_node *port_dn;
+ struct dsa_port *dport;
int ret, port;
for (port = 0; port < DSA_MAX_PORTS; port++) {
if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
continue;
- port_dn = ds->ports[port].dn;
- ret = dsa_cpu_dsa_setup(ds, dev, port_dn, port);
+ dport = &ds->ports[port];
+ ret = dsa_cpu_dsa_setup(ds, dev, dport, port);
if (ret)
return ret;
}
@@ -364,8 +365,10 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
return ds;
}
-void dsa_cpu_dsa_destroy(struct device_node *port_dn)
+void dsa_cpu_dsa_destroy(struct dsa_port *port)
{
+ struct device_node *port_dn = port->dn;
+
if (of_phy_is_fixed_link(port_dn))
of_phy_deregister_fixed_link(port_dn);
}
@@ -389,7 +392,7 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
for (port = 0; port < DSA_MAX_PORTS; port++) {
if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
continue;
- dsa_cpu_dsa_destroy(ds->ports[port].dn);
+ dsa_cpu_dsa_destroy(&ds->ports[port]);
/* Clearing a bit which is not set does no harm */
ds->cpu_port_mask |= ~(1 << port);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 2cf489c5e90f..56c43ca7c049 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -78,14 +78,19 @@ static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
kref_put(&dst->refcount, dsa_free_dst);
}
-static bool dsa_port_is_dsa(struct device_node *port)
+static bool dsa_port_is_valid(struct dsa_port *port)
{
- return !!of_parse_phandle(port, "link", 0);
+ return !!port->dn;
}
-static bool dsa_port_is_cpu(struct device_node *port)
+static bool dsa_port_is_dsa(struct dsa_port *port)
{
- return !!of_parse_phandle(port, "ethernet", 0);
+ return !!of_parse_phandle(port->dn, "link", 0);
+}
+
+static bool dsa_port_is_cpu(struct dsa_port *port)
+{
+ return !!of_parse_phandle(port->dn, "ethernet", 0);
}
static bool dsa_ds_find_port(struct dsa_switch *ds,
@@ -119,7 +124,7 @@ static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
static int dsa_port_complete(struct dsa_switch_tree *dst,
struct dsa_switch *src_ds,
- struct device_node *port,
+ struct dsa_port *port,
u32 src_port)
{
struct device_node *link;
@@ -127,7 +132,7 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
struct dsa_switch *dst_ds;
for (index = 0;; index++) {
- link = of_parse_phandle(port, "link", index);
+ link = of_parse_phandle(port->dn, "link", index);
if (!link)
break;
@@ -150,13 +155,13 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
*/
static int dsa_ds_complete(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
int err;
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (!dsa_port_is_dsa(port))
@@ -196,7 +201,7 @@ static int dsa_dst_complete(struct dsa_switch_tree *dst)
return 0;
}
-static int dsa_dsa_port_apply(struct device_node *port, u32 index,
+static int dsa_dsa_port_apply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
int err;
@@ -211,13 +216,13 @@ static int dsa_dsa_port_apply(struct device_node *port, u32 index,
return 0;
}
-static void dsa_dsa_port_unapply(struct device_node *port, u32 index,
+static void dsa_dsa_port_unapply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
dsa_cpu_dsa_destroy(port);
}
-static int dsa_cpu_port_apply(struct device_node *port, u32 index,
+static int dsa_cpu_port_apply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
int err;
@@ -234,7 +239,7 @@ static int dsa_cpu_port_apply(struct device_node *port, u32 index,
return 0;
}
-static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
+static void dsa_cpu_port_unapply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
dsa_cpu_dsa_destroy(port);
@@ -242,13 +247,13 @@ static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
}
-static int dsa_user_port_apply(struct device_node *port, u32 index,
+static int dsa_user_port_apply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
const char *name;
int err;
- name = of_get_property(port, "label", NULL);
+ name = of_get_property(port->dn, "label", NULL);
if (!name)
name = "eth%d";
@@ -262,7 +267,7 @@ static int dsa_user_port_apply(struct device_node *port, u32 index,
return 0;
}
-static void dsa_user_port_unapply(struct device_node *port, u32 index,
+static void dsa_user_port_unapply(struct dsa_port *port, u32 index,
struct dsa_switch *ds)
{
if (ds->ports[index].netdev) {
@@ -274,7 +279,7 @@ static void dsa_user_port_unapply(struct device_node *port, u32 index,
static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
int err;
@@ -308,8 +313,8 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
}
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (dsa_port_is_dsa(port)) {
@@ -336,12 +341,12 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
static void dsa_ds_unapply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (dsa_port_is_dsa(port)) {
@@ -425,7 +430,7 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
dst->applied = false;
}
-static int dsa_cpu_parse(struct device_node *port, u32 index,
+static int dsa_cpu_parse(struct dsa_port *port, u32 index,
struct dsa_switch_tree *dst,
struct dsa_switch *ds)
{
@@ -433,7 +438,7 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
struct net_device *ethernet_dev;
struct device_node *ethernet;
- ethernet = of_parse_phandle(port, "ethernet", 0);
+ ethernet = of_parse_phandle(port->dn, "ethernet", 0);
if (!ethernet)
return -EINVAL;
@@ -466,13 +471,13 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
{
- struct device_node *port;
+ struct dsa_port *port;
u32 index;
int err;
for (index = 0; index < DSA_MAX_PORTS; index++) {
- port = ds->ports[index].dn;
- if (!port)
+ port = &ds->ports[index];
+ if (!dsa_port_is_valid(port))
continue;
if (dsa_port_is_cpu(port)) {
@@ -533,7 +538,7 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
* to have access to a correct value, just like what
* net/dsa/dsa.c::dsa_switch_setup_one does.
*/
- if (!dsa_port_is_cpu(port))
+ if (!dsa_port_is_cpu(&ds->ports[reg]))
ds->enabled_port_mask |= 1 << reg;
}
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 63ae1484abae..16194a4bb2fe 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -50,8 +50,8 @@ struct dsa_slave_priv {
/* dsa.c */
int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
- struct device_node *port_dn, int port);
-void dsa_cpu_dsa_destroy(struct device_node *port_dn);
+ struct dsa_port *dport, int port);
+void dsa_cpu_dsa_destroy(struct dsa_port *dport);
const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
int dsa_cpu_port_ethtool_setup(struct dsa_switch *ds);
void dsa_cpu_port_ethtool_restore(struct dsa_switch *ds);
--
2.9.3
^ permalink raw reply related
* [PATCH net-next 0/4] net: dsa: Preparatory patches
From: Florian Fainelli @ 2017-01-26 18:28 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
Hi David,
This patch series extracts the 4 patches of the larger: net: dsa: Support for
pdata in dsa2 while we wait for feedback from Greg KH on the device references.
Thanks!
Florian Fainelli (4):
net: dsa: Pass device pointer to dsa_register_switch
net: dsa: Make most functions take a dsa_port argument
net: dsa: Suffix function manipulating device_node with _dn
net: dsa: Move ports assignment closer to error checking
drivers/net/dsa/b53/b53_common.c | 2 +-
drivers/net/dsa/mv88e6xxx/chip.c | 11 ++---
drivers/net/dsa/qca8k.c | 2 +-
include/net/dsa.h | 2 +-
net/dsa/dsa.c | 15 ++++---
net/dsa/dsa2.c | 87 ++++++++++++++++++++++------------------
net/dsa/dsa_priv.h | 4 +-
7 files changed, 67 insertions(+), 56 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH net-next 1/4] net: dsa: Pass device pointer to dsa_register_switch
From: Florian Fainelli @ 2017-01-26 18:28 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170126182808.13435-1-f.fainelli@gmail.com>
In preparation for allowing dsa_register_switch() to be supplied with
device/platform data, pass down a struct device pointer instead of a
struct device_node.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 2 +-
drivers/net/dsa/mv88e6xxx/chip.c | 11 ++++++-----
drivers/net/dsa/qca8k.c | 2 +-
include/net/dsa.h | 2 +-
net/dsa/dsa2.c | 7 ++++---
5 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 5cbb14f6a03b..bb210b12ad1b 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1894,7 +1894,7 @@ int b53_switch_register(struct b53_device *dev)
pr_info("found switch: %s, rev %i\n", dev->name, dev->core_rev);
- return dsa_register_switch(dev->ds, dev->ds->dev->of_node);
+ return dsa_register_switch(dev->ds, dev->ds->dev);
}
EXPORT_SYMBOL(b53_switch_register);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 5668e778ed1d..12e58ec94b7a 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4356,8 +4356,7 @@ static struct dsa_switch_driver mv88e6xxx_switch_drv = {
.ops = &mv88e6xxx_switch_ops,
};
-static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip,
- struct device_node *np)
+static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
{
struct device *dev = chip->dev;
struct dsa_switch *ds;
@@ -4372,7 +4371,7 @@ static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip,
dev_set_drvdata(dev, ds);
- return dsa_register_switch(ds, np);
+ return dsa_register_switch(ds, dev);
}
static void mv88e6xxx_unregister_switch(struct mv88e6xxx_chip *chip)
@@ -4456,9 +4455,11 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
if (err)
goto out_g2_irq;
- err = mv88e6xxx_register_switch(chip, np);
- if (err)
+ err = mv88e6xxx_register_switch(chip);
+ if (err) {
+ mv88e6xxx_mdio_unregister(chip);
goto out_mdio;
+ }
return 0;
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 54d270d59eb0..c084aa484d2b 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -964,7 +964,7 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
mutex_init(&priv->reg_mutex);
dev_set_drvdata(&mdiodev->dev, priv);
- return dsa_register_switch(priv->ds, priv->ds->dev->of_node);
+ return dsa_register_switch(priv->ds, &mdiodev->dev);
}
static void
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 08b340403927..92fd795e9573 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -387,7 +387,7 @@ static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
}
void dsa_unregister_switch(struct dsa_switch *ds);
-int dsa_register_switch(struct dsa_switch *ds, struct device_node *np);
+int dsa_register_switch(struct dsa_switch *ds, struct device *dev);
#ifdef CONFIG_PM_SLEEP
int dsa_switch_suspend(struct dsa_switch *ds);
int dsa_switch_resume(struct dsa_switch *ds);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 866222a8f9bf..2cf489c5e90f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -578,8 +578,9 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
return ports;
}
-static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
{
+ struct device_node *np = dev->of_node;
struct device_node *ports = dsa_get_ports(ds, np);
struct dsa_switch_tree *dst;
u32 tree, index;
@@ -659,12 +660,12 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
return err;
}
-int dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+int dsa_register_switch(struct dsa_switch *ds, struct device *dev)
{
int err;
mutex_lock(&dsa2_mutex);
- err = _dsa_register_switch(ds, np);
+ err = _dsa_register_switch(ds, dev);
mutex_unlock(&dsa2_mutex);
return err;
--
2.9.3
^ permalink raw reply related
* [PATCH net-next 3/4] net: dsa: Suffix function manipulating device_node with _dn
From: Florian Fainelli @ 2017-01-26 18:28 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170126182808.13435-1-f.fainelli@gmail.com>
Make it clear that these functions take a device_node structure pointer
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa2.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 56c43ca7c049..4c11619a818b 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -93,8 +93,8 @@ static bool dsa_port_is_cpu(struct dsa_port *port)
return !!of_parse_phandle(port->dn, "ethernet", 0);
}
-static bool dsa_ds_find_port(struct dsa_switch *ds,
- struct device_node *port)
+static bool dsa_ds_find_port_dn(struct dsa_switch *ds,
+ struct device_node *port)
{
u32 index;
@@ -104,8 +104,8 @@ static bool dsa_ds_find_port(struct dsa_switch *ds,
return false;
}
-static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
- struct device_node *port)
+static struct dsa_switch *dsa_dst_find_port_dn(struct dsa_switch_tree *dst,
+ struct device_node *port)
{
struct dsa_switch *ds;
u32 index;
@@ -115,7 +115,7 @@ static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
if (!ds)
continue;
- if (dsa_ds_find_port(ds, port))
+ if (dsa_ds_find_port_dn(ds, port))
return ds;
}
@@ -136,7 +136,7 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
if (!link)
break;
- dst_ds = dsa_dst_find_port(dst, link);
+ dst_ds = dsa_dst_find_port_dn(dst, link);
of_node_put(link);
if (!dst_ds)
@@ -545,7 +545,7 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
return 0;
}
-static int dsa_parse_member(struct device_node *np, u32 *tree, u32 *index)
+static int dsa_parse_member_dn(struct device_node *np, u32 *tree, u32 *index)
{
int err;
@@ -591,7 +591,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
u32 tree, index;
int i, err;
- err = dsa_parse_member(np, &tree, &index);
+ err = dsa_parse_member_dn(np, &tree, &index);
if (err)
return err;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v1] net: phy: micrel: add KSZ8795 ethernet switch
From: Florian Fainelli @ 2017-01-26 18:36 UTC (permalink / raw)
To: Sean Nyekjær, Andrew Lunn; +Cc: netdev
In-Reply-To: <732c14ff-f0cf-67d5-bcf9-cbbf9a4391b3@prevas.dk>
On 01/26/2017 09:51 AM, Sean Nyekjær wrote:
>
>
> On 2017-01-23 16:12, Andrew Lunn wrote:
>> On Mon, Jan 23, 2017 at 08:58:46AM +0100, Sean Nyekjaer wrote:
>>> This is add support for the PHYs in the KSZ8795 5port managed switch.
>>>
>>> It will allow to detect the link between the switch and the soc
>>> and uses the same read_status functions as the KSZ8873MLL switch.
>>>
>>> This ethernet switch have unfortunately the same phy id as KSZ8051.
>>>
>>> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
>>> ---
>>> drivers/net/phy/micrel.c | 14 ++++++++++++++
>>> include/linux/micrel_phy.h | 2 ++
>>> 2 files changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>>> index ea92d524d5a8..fa158ae5115b 100644
>>> --- a/drivers/net/phy/micrel.c
>>> +++ b/drivers/net/phy/micrel.c
>>> @@ -1014,6 +1014,20 @@ static struct phy_driver ksphy_driver[] = {
>>> .get_stats = kszphy_get_stats,
>>> .suspend = genphy_suspend,
>>> .resume = genphy_resume,
>>> +}, {
>>> + .phy_id = PHY_ID_KSZ8795,
>>> + .phy_id_mask = MICREL_PHY_ID_MASK,
>>> + .name = "Micrel KSZ8795 Switch",
>> This name is confusing. You are adding support for the PHYs embedded
>> in the switch, not the switch itself. The phylib has no idea these
>> PHYs are inside a switch. It does not matter. It is just a PHY. Please
>> drop the word "Switch".
>>
>> Thanks
>> Andrew
> Okay i'll understand :-)
> I remove the word "Switch" from the commit message and phy name.
> The rest is okay?
> What is generally being done when to phy's have the same id, micrel have
> done this many times...
You need unique names, we have a case like that in
drivers/net/phy/bcm7xxx.c for the 7439 PHY where it exists with two
different IDs, so the first one is named "Broadcom BCM7439" and the
second one is named "Broadcom BCM7439 (2)", feel free to adopt the same
convention, or something else.
The rest of your patch is okay.
--
Florian
^ 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