Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v4 0/5] vsock/virtio: optimizations to increase the throughput
From: Michael S. Tsirkin @ 2019-07-29 13:59 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: netdev, linux-kernel, Stefan Hajnoczi, David S. Miller,
	virtualization, Jason Wang, kvm
In-Reply-To: <20190717113030.163499-1-sgarzare@redhat.com>

On Wed, Jul 17, 2019 at 01:30:25PM +0200, Stefano Garzarella wrote:
> This series tries to increase the throughput of virtio-vsock with slight
> changes.
> While I was testing the v2 of this series I discovered an huge use of memory,
> so I added patch 1 to mitigate this issue. I put it in this series in order
> to better track the performance trends.

Series:

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Can this go into net-next?


> v4:
> - rebased all patches on current master (conflicts is Patch 4)
> - Patch 1: added Stefan's R-b
> - Patch 3: removed lock when buf_alloc is written [David];
>            moved this patch after "vsock/virtio: reduce credit update messages"
>            to make it clearer
> - Patch 4: vhost_exceeds_weight() is recently introduced, so I've solved some
>            conflicts
> 
> v3: https://patchwork.kernel.org/cover/10970145
> 
> v2: https://patchwork.kernel.org/cover/10938743
> 
> v1: https://patchwork.kernel.org/cover/10885431
> 
> Below are the benchmarks step by step. I used iperf3 [1] modified with VSOCK
> support. As Micheal suggested in the v1, I booted host and guest with 'nosmap'.
> 
> A brief description of patches:
> - Patches 1:   limit the memory usage with an extra copy for small packets
> - Patches 2+3: reduce the number of credit update messages sent to the
>                transmitter
> - Patches 4+5: allow the host to split packets on multiple buffers and use
>                VIRTIO_VSOCK_MAX_PKT_BUF_SIZE as the max packet size allowed
> 
>                     host -> guest [Gbps]
> pkt_size before opt   p 1     p 2+3    p 4+5
> 
> 32         0.032     0.030    0.048    0.051
> 64         0.061     0.059    0.108    0.117
> 128        0.122     0.112    0.227    0.234
> 256        0.244     0.241    0.418    0.415
> 512        0.459     0.466    0.847    0.865
> 1K         0.927     0.919    1.657    1.641
> 2K         1.884     1.813    3.262    3.269
> 4K         3.378     3.326    6.044    6.195
> 8K         5.637     5.676   10.141   11.287
> 16K        8.250     8.402   15.976   16.736
> 32K       13.327    13.204   19.013   20.515
> 64K       21.241    21.341   20.973   21.879
> 128K      21.851    22.354   21.816   23.203
> 256K      21.408    21.693   21.846   24.088
> 512K      21.600    21.899   21.921   24.106
> 
>                     guest -> host [Gbps]
> pkt_size before opt   p 1     p 2+3    p 4+5
> 
> 32         0.045     0.046    0.057    0.057
> 64         0.089     0.091    0.103    0.104
> 128        0.170     0.179    0.192    0.200
> 256        0.364     0.351    0.361    0.379
> 512        0.709     0.699    0.731    0.790
> 1K         1.399     1.407    1.395    1.427
> 2K         2.670     2.684    2.745    2.835
> 4K         5.171     5.199    5.305    5.451
> 8K         8.442     8.500   10.083    9.941
> 16K       12.305    12.259   13.519   15.385
> 32K       11.418    11.150   11.988   24.680
> 64K       10.778    10.659   11.589   35.273
> 128K      10.421    10.339   10.939   40.338
> 256K      10.300     9.719   10.508   36.562
> 512K       9.833     9.808   10.612   35.979
> 
> As Stefan suggested in the v1, I measured also the efficiency in this way:
>     efficiency = Mbps / (%CPU_Host + %CPU_Guest)
> 
> The '%CPU_Guest' is taken inside the VM. I know that it is not the best way,
> but it's provided for free from iperf3 and could be an indication.
> 
>         host -> guest efficiency [Mbps / (%CPU_Host + %CPU_Guest)]
> pkt_size before opt   p 1     p 2+3    p 4+5
> 
> 32         0.35      0.45     0.79     1.02
> 64         0.56      0.80     1.41     1.54
> 128        1.11      1.52     3.03     3.12
> 256        2.20      2.16     5.44     5.58
> 512        4.17      4.18    10.96    11.46
> 1K         8.30      8.26    20.99    20.89
> 2K        16.82     16.31    39.76    39.73
> 4K        30.89     30.79    74.07    75.73
> 8K        53.74     54.49   124.24   148.91
> 16K       80.68     83.63   200.21   232.79
> 32K      132.27    132.52   260.81   357.07
> 64K      229.82    230.40   300.19   444.18
> 128K     332.60    329.78   331.51   492.28
> 256K     331.06    337.22   339.59   511.59
> 512K     335.58    328.50   331.56   504.56
> 
>         guest -> host efficiency [Mbps / (%CPU_Host + %CPU_Guest)]
> pkt_size before opt   p 1     p 2+3    p 4+5
> 
> 32         0.43      0.43     0.53     0.56
> 64         0.85      0.86     1.04     1.10
> 128        1.63      1.71     2.07     2.13
> 256        3.48      3.35     4.02     4.22
> 512        6.80      6.67     7.97     8.63
> 1K        13.32     13.31    15.72    15.94
> 2K        25.79     25.92    30.84    30.98
> 4K        50.37     50.48    58.79    59.69
> 8K        95.90     96.15   107.04   110.33
> 16K      145.80    145.43   143.97   174.70
> 32K      147.06    144.74   146.02   282.48
> 64K      145.25    143.99   141.62   406.40
> 128K     149.34    146.96   147.49   489.34
> 256K     156.35    149.81   152.21   536.37
> 512K     151.65    150.74   151.52   519.93
> 
> [1] https://github.com/stefano-garzarella/iperf/
> 
> Stefano Garzarella (5):
>   vsock/virtio: limit the memory used per-socket
>   vsock/virtio: reduce credit update messages
>   vsock/virtio: fix locking in virtio_transport_inc_tx_pkt()
>   vhost/vsock: split packets to send using multiple buffers
>   vsock/virtio: change the maximum packet size allowed
> 
>  drivers/vhost/vsock.c                   | 68 ++++++++++++-----
>  include/linux/virtio_vsock.h            |  4 +-
>  net/vmw_vsock/virtio_transport.c        |  1 +
>  net/vmw_vsock/virtio_transport_common.c | 99 ++++++++++++++++++++-----
>  4 files changed, 134 insertions(+), 38 deletions(-)
> 
> -- 
> 2.20.1

^ permalink raw reply

* Re: [PATCH v4 1/5] vsock/virtio: limit the memory used per-socket
From: Michael S. Tsirkin @ 2019-07-29 14:04 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: netdev, linux-kernel, Stefan Hajnoczi, David S. Miller,
	virtualization, Jason Wang, kvm
In-Reply-To: <20190717113030.163499-2-sgarzare@redhat.com>

On Wed, Jul 17, 2019 at 01:30:26PM +0200, Stefano Garzarella wrote:
> Since virtio-vsock was introduced, the buffers filled by the host
> and pushed to the guest using the vring, are directly queued in
> a per-socket list. These buffers are preallocated by the guest
> with a fixed size (4 KB).
> 
> The maximum amount of memory used by each socket should be
> controlled by the credit mechanism.
> The default credit available per-socket is 256 KB, but if we use
> only 1 byte per packet, the guest can queue up to 262144 of 4 KB
> buffers, using up to 1 GB of memory per-socket. In addition, the
> guest will continue to fill the vring with new 4 KB free buffers
> to avoid starvation of other sockets.
> 
> This patch mitigates this issue copying the payload of small
> packets (< 128 bytes) into the buffer of last packet queued, in
> order to avoid wasting memory.
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

This is good enough for net-next, but for net I think we
should figure out how to address the issue completely.
Can we make the accounting precise? What happens to
performance if we do?


> ---
>  drivers/vhost/vsock.c                   |  2 +
>  include/linux/virtio_vsock.h            |  1 +
>  net/vmw_vsock/virtio_transport.c        |  1 +
>  net/vmw_vsock/virtio_transport_common.c | 60 +++++++++++++++++++++----
>  4 files changed, 55 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 6a50e1d0529c..6c8390a2af52 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -329,6 +329,8 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq,
>  		return NULL;
>  	}
>  
> +	pkt->buf_len = pkt->len;
> +
>  	nbytes = copy_from_iter(pkt->buf, pkt->len, &iov_iter);
>  	if (nbytes != pkt->len) {
>  		vq_err(vq, "Expected %u byte payload, got %zu bytes\n",
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index e223e2632edd..7d973903f52e 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -52,6 +52,7 @@ struct virtio_vsock_pkt {
>  	/* socket refcnt not held, only use for cancellation */
>  	struct vsock_sock *vsk;
>  	void *buf;
> +	u32 buf_len;
>  	u32 len;
>  	u32 off;
>  	bool reply;
> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index 0815d1357861..082a30936690 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -307,6 +307,7 @@ static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)
>  			break;
>  		}
>  
> +		pkt->buf_len = buf_len;
>  		pkt->len = buf_len;
>  
>  		sg_init_one(&hdr, &pkt->hdr, sizeof(pkt->hdr));
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 6f1a8aff65c5..095221f94786 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -26,6 +26,9 @@
>  /* How long to wait for graceful shutdown of a connection */
>  #define VSOCK_CLOSE_TIMEOUT (8 * HZ)
>  
> +/* Threshold for detecting small packets to copy */
> +#define GOOD_COPY_LEN  128
> +
>  static const struct virtio_transport *virtio_transport_get_ops(void)
>  {
>  	const struct vsock_transport *t = vsock_core_get_transport();
> @@ -64,6 +67,9 @@ virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info,
>  		pkt->buf = kmalloc(len, GFP_KERNEL);
>  		if (!pkt->buf)
>  			goto out_pkt;
> +
> +		pkt->buf_len = len;
> +
>  		err = memcpy_from_msg(pkt->buf, info->msg, len);
>  		if (err)
>  			goto out;
> @@ -841,24 +847,60 @@ virtio_transport_recv_connecting(struct sock *sk,
>  	return err;
>  }
>  
> +static void
> +virtio_transport_recv_enqueue(struct vsock_sock *vsk,
> +			      struct virtio_vsock_pkt *pkt)
> +{
> +	struct virtio_vsock_sock *vvs = vsk->trans;
> +	bool free_pkt = false;
> +
> +	pkt->len = le32_to_cpu(pkt->hdr.len);
> +	pkt->off = 0;
> +
> +	spin_lock_bh(&vvs->rx_lock);
> +
> +	virtio_transport_inc_rx_pkt(vvs, pkt);
> +
> +	/* Try to copy small packets into the buffer of last packet queued,
> +	 * to avoid wasting memory queueing the entire buffer with a small
> +	 * payload.
> +	 */
> +	if (pkt->len <= GOOD_COPY_LEN && !list_empty(&vvs->rx_queue)) {
> +		struct virtio_vsock_pkt *last_pkt;
> +
> +		last_pkt = list_last_entry(&vvs->rx_queue,
> +					   struct virtio_vsock_pkt, list);
> +
> +		/* If there is space in the last packet queued, we copy the
> +		 * new packet in its buffer.
> +		 */
> +		if (pkt->len <= last_pkt->buf_len - last_pkt->len) {
> +			memcpy(last_pkt->buf + last_pkt->len, pkt->buf,
> +			       pkt->len);
> +			last_pkt->len += pkt->len;
> +			free_pkt = true;
> +			goto out;
> +		}
> +	}
> +
> +	list_add_tail(&pkt->list, &vvs->rx_queue);
> +
> +out:
> +	spin_unlock_bh(&vvs->rx_lock);
> +	if (free_pkt)
> +		virtio_transport_free_pkt(pkt);
> +}
> +
>  static int
>  virtio_transport_recv_connected(struct sock *sk,
>  				struct virtio_vsock_pkt *pkt)
>  {
>  	struct vsock_sock *vsk = vsock_sk(sk);
> -	struct virtio_vsock_sock *vvs = vsk->trans;
>  	int err = 0;
>  
>  	switch (le16_to_cpu(pkt->hdr.op)) {
>  	case VIRTIO_VSOCK_OP_RW:
> -		pkt->len = le32_to_cpu(pkt->hdr.len);
> -		pkt->off = 0;
> -
> -		spin_lock_bh(&vvs->rx_lock);
> -		virtio_transport_inc_rx_pkt(vvs, pkt);
> -		list_add_tail(&pkt->list, &vvs->rx_queue);
> -		spin_unlock_bh(&vvs->rx_lock);
> -
> +		virtio_transport_recv_enqueue(vsk, pkt);
>  		sk->sk_data_ready(sk);
>  		return err;
>  	case VIRTIO_VSOCK_OP_CREDIT_UPDATE:
> -- 
> 2.20.1

^ permalink raw reply

* Re: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions
From: Jason Gunthorpe @ 2019-07-29 14:04 UTC (permalink / raw)
  To: Gal Pressman
  Cc: Michal Kalderon, Ariel Elior, dledford@redhat.com,
	linux-rdma@vger.kernel.org, davem@davemloft.net,
	netdev@vger.kernel.org
In-Reply-To: <d632598e-0896-fa10-9148-73794a9a49d7@amazon.com>

On Mon, Jul 29, 2019 at 04:53:38PM +0300, Gal Pressman wrote:
> On 29/07/2019 15:58, Michal Kalderon wrote:
> >> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> >> owner@vger.kernel.org> On Behalf Of Jason Gunthorpe
> >>
> >>> +	xa_lock(&ucontext->mmap_xa);
> >>> +	if (check_add_overflow(ucontext->mmap_xa_page,
> >>> +			       (u32)(length >> PAGE_SHIFT),
> >>> +			       &next_mmap_page))
> >>> +		goto err_unlock;
> >>
> >> I still don't like that this algorithm latches into a permanent failure when the
> >> xa_page wraps.
> >>
> >> It seems worth spending a bit more time here to tidy this.. Keep using the
> >> mmap_xa_page scheme, but instead do something like
> >>
> >> alloc_cyclic_range():
> >>
> >> while () {
> >>    // Find first empty element in a cyclic way
> >>    xa_page_first = mmap_xa_page;
> >>    xa_find(xa, &xa_page_first, U32_MAX, XA_FREE_MARK)
> >>
> >>    // Is there a enough room to have the range?
> >>    if (check_add_overflow(xa_page_first, npages, &xa_page_end)) {
> >>       mmap_xa_page = 0;
> >>       continue;
> >>    }
> >>
> >>    // See if the element before intersects
> >>    elm = xa_find(xa, &zero, xa_page_end, 0);
> >>    if (elm && intersects(xa_page_first, xa_page_last, elm->first, elm->last)) {
> >>       mmap_xa_page = elm->last + 1;
> >>       continue
> >>    }
> >>
> >>    // xa_page_first -> xa_page_end should now be free
> >>    xa_insert(xa, xa_page_start, entry);
> >>    mmap_xa_page = xa_page_end + 1;
> >>    return xa_page_start;
> >> }
> >>
> >> Approximately, please check it.
> > Gal & Jason, 
> > 
> > Coming back to the mmap_xa_page algorithm. I couldn't find some background on this. 
> > Why do you need the length to be represented in the mmap_xa_page ?  
> > Why not simply use xa_alloc_cyclic ( like in siw )

I think siw is dealing with only PAGE_SIZE objects, efa had variable
sized ones.

> > This is simply a key to a mmap object... 
> 
> The intention was that the entry would "occupy" number of xarray elements
> according to its size (in pages). It wasn't initially like this, but IIRC this
> was preferred by Jason.

It is not so critical, maybe we could drop it if it is really
simplifiying. But it doesn't look so hard to make an xa algorithm that
will be OK.

The offset/length is shown in things like lsof and what not, and from
a debugging perspective it makes a lot more sense if the offset/length
are sensible, ie they should not overlap.

Jason

^ permalink raw reply

* Re: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions
From: Jason Gunthorpe @ 2019-07-29 14:06 UTC (permalink / raw)
  To: Gal Pressman
  Cc: Michal Kalderon, Kamal Heib, Ariel Elior, dledford@redhat.com,
	linux-rdma@vger.kernel.org, davem@davemloft.net,
	netdev@vger.kernel.org
In-Reply-To: <1e54c4de-7349-3154-1b98-39774c83899f@amazon.com>

On Sun, Jul 28, 2019 at 11:45:56AM +0300, Gal Pressman wrote:
> On 26/07/2019 16:23, Jason Gunthorpe wrote:
> > On Fri, Jul 26, 2019 at 08:42:07AM +0000, Michal Kalderon wrote:
> > 
> >>>> But we don't free entires from the xa_array ( only when ucontext is
> >>>> destroyed) so how will There be an empty element after we wrap ?
> >>>
> >>> Oh!
> >>>
> >>> That should be fixed up too, in the general case if a user is
> >>> creating/destroying driver objects in loop we don't want memory usage to
> >>> be unbounded.
> >>>
> >>> The rdma_user_mmap stuff has VMA ops that can refcount the xa entry and
> >>> now that this is core code it is easy enough to harmonize the two things and
> >>> track the xa side from the struct rdma_umap_priv
> >>>
> >>> The question is, does EFA or qedr have a use model for this that allows a
> >>> userspace verb to create/destroy in a loop? ie do we need to fix this right
> >>> now?
> > 
> >> The mapping occurs for every qp and cq creation. So yes.
> >>
> >> So do you mean add a ref-cnt to the xarray entry and from umap
> >> decrease the refcnt and free?
> > 
> > Yes, free the entry (release the HW resource) and release the xa_array
> > ID.
> 
> This is a bit tricky for EFA.
> The UAR BAR resources (LLQ for example) aren't cleaned up until the UAR is
> deallocated, so many of the entries won't really be freed when the refcount
> reaches zero (i.e the HW considers these entries as refcounted as long as the
> UAR exists). The best we can do is free the DMA buffers for appropriate entries.

Drivers can still defer HW destruction until the ucontext destroys,
but this gives an option to move it sooner, which looks like the other
drivers do need as they can allocate these things in userspace loops.

Jason

^ permalink raw reply

* RE: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions
From: Michal Kalderon @ 2019-07-29 14:07 UTC (permalink / raw)
  To: Gal Pressman, Jason Gunthorpe
  Cc: Ariel Elior, dledford@redhat.com, linux-rdma@vger.kernel.org,
	davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <d632598e-0896-fa10-9148-73794a9a49d7@amazon.com>

> From: Gal Pressman <galpress@amazon.com>
> Sent: Monday, July 29, 2019 4:54 PM
> 
> On 29/07/2019 15:58, Michal Kalderon wrote:
> >> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> >> owner@vger.kernel.org> On Behalf Of Jason Gunthorpe
> >>
> >>> +	xa_lock(&ucontext->mmap_xa);
> >>> +	if (check_add_overflow(ucontext->mmap_xa_page,
> >>> +			       (u32)(length >> PAGE_SHIFT),
> >>> +			       &next_mmap_page))
> >>> +		goto err_unlock;
> >>
> >> I still don't like that this algorithm latches into a permanent
> >> failure when the xa_page wraps.
> >>
> >> It seems worth spending a bit more time here to tidy this.. Keep
> >> using the mmap_xa_page scheme, but instead do something like
> >>
> >> alloc_cyclic_range():
> >>
> >> while () {
> >>    // Find first empty element in a cyclic way
> >>    xa_page_first = mmap_xa_page;
> >>    xa_find(xa, &xa_page_first, U32_MAX, XA_FREE_MARK)
> >>
> >>    // Is there a enough room to have the range?
> >>    if (check_add_overflow(xa_page_first, npages, &xa_page_end)) {
> >>       mmap_xa_page = 0;
> >>       continue;
> >>    }
> >>
> >>    // See if the element before intersects
> >>    elm = xa_find(xa, &zero, xa_page_end, 0);
> >>    if (elm && intersects(xa_page_first, xa_page_last, elm->first, elm-
> >last)) {
> >>       mmap_xa_page = elm->last + 1;
> >>       continue
> >>    }
> >>
> >>    // xa_page_first -> xa_page_end should now be free
> >>    xa_insert(xa, xa_page_start, entry);
> >>    mmap_xa_page = xa_page_end + 1;
> >>    return xa_page_start;
> >> }
> >>
> >> Approximately, please check it.
> > Gal & Jason,
> >
> > Coming back to the mmap_xa_page algorithm. I couldn't find some
> background on this.
> > Why do you need the length to be represented in the mmap_xa_page ?
> > Why not simply use xa_alloc_cyclic ( like in siw ) This is simply a
> > key to a mmap object...
> 
> The intention was that the entry would "occupy" number of xarray elements
> according to its size (in pages). It wasn't initially like this, but IIRC this was
> preferred by Jason.

Thanks, so Jason, if we're now freeing the objects, can we simply us xa_alloc_cyclic instead? 
Thanks,
Michal

^ permalink raw reply

* RE: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jose Abreu @ 2019-07-29 14:08 UTC (permalink / raw)
  To: Robin Murphy, Jose Abreu, Jon Hunter,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org, Catalin Marinas,
	Will Deacon
  Cc: Joao Pinto, Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai,
	Maxime Coquelin, linux-tegra, Giuseppe Cavallaro,
	David S . Miller
In-Reply-To: <fcf648d2-70cc-d734-871a-ca7f745791b7@arm.com>

[-- Attachment #1: Type: text/plain, Size: 4612 bytes --]

From: Robin Murphy <robin.murphy@arm.com>
Date: Jul/29/2019, 12:52:02 (UTC+00:00)

> On 29/07/2019 12:29, Jose Abreu wrote:
> > ++ Catalin, Will (ARM64 Maintainers)
> > 
> > From: Jon Hunter <jonathanh@nvidia.com>
> > Date: Jul/29/2019, 11:55:18 (UTC+00:00)
> > 
> >>
> >> On 29/07/2019 09:16, Jose Abreu wrote:
> >>> From: Jose Abreu <joabreu@synopsys.com>
> >>> Date: Jul/27/2019, 16:56:37 (UTC+00:00)
> >>>
> >>>> From: Jon Hunter <jonathanh@nvidia.com>
> >>>> Date: Jul/26/2019, 15:11:00 (UTC+00:00)
> >>>>
> >>>>>
> >>>>> On 25/07/2019 16:12, Jose Abreu wrote:
> >>>>>> From: Jon Hunter <jonathanh@nvidia.com>
> >>>>>> Date: Jul/25/2019, 15:25:59 (UTC+00:00)
> >>>>>>
> >>>>>>>
> >>>>>>> On 25/07/2019 14:26, Jose Abreu wrote:
> >>>>>>>
> >>>>>>> ...
> >>>>>>>
> >>>>>>>> Well, I wasn't expecting that :/
> >>>>>>>>
> >>>>>>>> Per documentation of barriers I think we should set descriptor fields
> >>>>>>>> and then barrier and finally ownership to HW so that remaining fields
> >>>>>>>> are coherent before owner is set.
> >>>>>>>>
> >>>>>>>> Anyway, can you also add a dma_rmb() after the call to
> >>>>>>>> stmmac_rx_status() ?
> >>>>>>>
> >>>>>>> Yes. I removed the debug print added the barrier, but that did not help.
> >>>>>>
> >>>>>> So, I was finally able to setup NFS using your replicated setup and I
> >>>>>> can't see the issue :(
> >>>>>>
> >>>>>> The only difference I have from yours is that I'm using TCP in NFS
> >>>>>> whilst you (I believe from the logs), use UDP.
> >>>>>
> >>>>> So I tried TCP by setting the kernel boot params to 'nfsvers=3' and
> >>>>> 'proto=tcp' and this does appear to be more stable, but not 100% stable.
> >>>>> It still appears to fail in the same place about 50% of the time.
> >>>>>
> >>>>>> You do have flow control active right ? And your HW FIFO size is >= 4k ?
> >>>>>
> >>>>> How can I verify if flow control is active?
> >>>>
> >>>> You can check it by dumping register MTL_RxQ_Operation_Mode (0xd30).
> >>
> >> Where would be the appropriate place to dump this? After probe? Maybe
> >> best if you can share a code snippet of where to dump this.
> >>
> >>>> Can you also add IOMMU debug in file "drivers/iommu/iommu.c" ?
> >>
> >> You can find a boot log here:
> >>
> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__paste.ubuntu.com_p_qtRqtYKHGF_&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=NrxsR2etpZHGb7HkN4XdgaGmKM1XYyldihNPL6qVSv0&s=CMATEcHVoqZw4sIrNOXc7SFE_kV_5CO5EU21-yJez6c&e=
> >>
> >>> And, please try attached debug patch.
> >>
> >> With this patch it appears to boot fine. So far no issues seen.
> > 
> > Thank you for testing.
> > 
> > Hi Catalin and Will,
> > 
> > Sorry to add you in such a long thread but we are seeing a DMA issue
> > with stmmac driver in an ARM64 platform with IOMMU enabled.
> > 
> > The issue seems to be solved when buffers allocation for DMA based
> > transfers are *not* mapped with the DMA_ATTR_SKIP_CPU_SYNC flag *OR*
> > when IOMMU is disabled.
> > 
> > Notice that after transfer is done we do use
> > dma_sync_single_for_{cpu,device} and then we reuse *the same* page for
> > another transfer.
> > 
> > Can you please comment on whether DMA_ATTR_SKIP_CPU_SYNC can not be used
> > in ARM64 platforms with IOMMU ?
> 
> In terms of what they do, there should be no difference on arm64 between:
> 
> dma_map_page(..., dir);
> ...
> dma_unmap_page(..., dir);
> 
> and:
> 
> dma_map_page_attrs(..., dir, DMA_ATTR_SKIP_CPU_SYNC);
> dma_sync_single_for_device(..., dir);
> ...
> dma_sync_single_for_cpu(..., dir);
> dma_unmap_page_attrs(..., dir, DMA_ATTR_SKIP_CPU_SYNC);
> 
> provided that the first sync covers the whole buffer and any subsequent 
> ones cover at least the parts of the buffer which may have changed. Plus 
> for coherent hardware it's entirely moot either way.

Thanks for confirming. That's indeed what stmmac is doing when buffer is 
received by syncing the packet size to CPU.

> 
> Given Jon's previous findings, I would lean towards the idea that 
> performing the extra (redundant) cache maintenance plus barrier in 
> dma_unmap is mostly just perturbing timing in the same way as the debug 
> print which also made things seem OK.

Mikko said that Tegra186 is not coherent so we have to explicit flush 
pipeline but I don't understand why sync_single() is not doing it ...

Jon, can you please remove *all* debug prints, hacks, etc ... and test 
this one in attach with plain -net tree ?

---
Thanks,
Jose Miguel Abreu

[-- Attachment #2: 0001-net-stmmac-Flush-all-data-cache-in-RX-path.patch --]
[-- Type: application/octet-stream, Size: 1647 bytes --]

From 1b512c799cd896c7b609be512db7c477def43c6b Mon Sep 17 00:00:00 2001
Message-Id: <1b512c799cd896c7b609be512db7c477def43c6b.1564408914.git.joabreu@synopsys.com>
From: Jose Abreu <joabreu@synopsys.com>
Date: Mon, 29 Jul 2019 16:01:36 +0200
Subject: [PATCH net] net: stmmac: Flush all data cache in RX path

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 98b1a5c6d537..ed7f0d6bd0bc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -27,6 +27,7 @@
 #include <linux/if.h>
 #include <linux/if_vlan.h>
 #include <linux/dma-mapping.h>
+#include <linux/dma-noncoherent.h>
 #include <linux/slab.h>
 #include <linux/prefetch.h>
 #include <linux/pinctrl/consumer.h>
@@ -3420,6 +3421,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 				continue;
 			}
 
+			arch_dma_prep_coherent(buf->page, frame_len);
+
 			dma_sync_single_for_cpu(priv->device, buf->addr,
 						frame_len, DMA_FROM_DEVICE);
 			skb_copy_to_linear_data(skb, page_address(buf->page),
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions
From: Jason Gunthorpe @ 2019-07-29 14:11 UTC (permalink / raw)
  To: Kamal Heib
  Cc: Michal Kalderon, ariel.elior, dledford, galpress, linux-rdma,
	davem, netdev
In-Reply-To: <20190728093051.GB5250@kheib-workstation>

On Sun, Jul 28, 2019 at 12:30:51PM +0300, Kamal Heib wrote:
> > Maybe put this in ib_core_uverbs.c ?
> > 
> > Kamal, you've been tackling various cleanups, maybe making ib_uverbs
> > unloadable again is something you'd be keen on?
> >
> 
> Yes, Could you please give some background on that?

Most of it is my fault from being too careless, but the general notion
is that all of these

$ grep EXPORT_SYMBOL uverbs_main.c uverbs_cmd.c  uverbs_marshall.c  rdma_core.c uverbs_std_types*.c uverbs_uapi.c 
uverbs_main.c:EXPORT_SYMBOL(ib_uverbs_get_ucontext_file);
uverbs_main.c:EXPORT_SYMBOL(rdma_user_mmap_io);
uverbs_cmd.c:EXPORT_SYMBOL(flow_resources_alloc);
uverbs_cmd.c:EXPORT_SYMBOL(ib_uverbs_flow_resources_free);
uverbs_cmd.c:EXPORT_SYMBOL(flow_resources_add);
uverbs_marshall.c:EXPORT_SYMBOL(ib_copy_ah_attr_to_user);
uverbs_marshall.c:EXPORT_SYMBOL(ib_copy_qp_attr_to_user);
uverbs_marshall.c:EXPORT_SYMBOL(ib_copy_path_rec_to_user);
uverbs_marshall.c:EXPORT_SYMBOL(ib_copy_path_rec_from_user);
rdma_core.c:EXPORT_SYMBOL(uverbs_idr_class);
rdma_core.c:EXPORT_SYMBOL(uverbs_close_fd);
rdma_core.c:EXPORT_SYMBOL(uverbs_fd_class);
uverbs_std_types.c:EXPORT_SYMBOL(uverbs_destroy_def_handler);

Need to go into some 'ib_core uverbs support' .c file in the ib_core,
be moved to a header inline, or moved otherwise

Maybe it is now unrealistic that the uapi is so complicated, ie
uverbs_close_fd is just not easy to fixup..

Maybe the only ones that need fixing are ib_uverbs_get_ucontext_file
rdma_user_mmap_io as alot of drivers are entangled on those now.

The other stuff is much harder..

Jason

^ permalink raw reply

* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Nikolay Aleksandrov @ 2019-07-29 14:21 UTC (permalink / raw)
  To: Allan W. Nielsen
  Cc: Horatiu Vultur, roopa, davem, bridge, netdev, linux-kernel
In-Reply-To: <20190729135205.oiuthcyesal4b4ct@lx-anielsen.microsemi.net>

On 29/07/2019 16:52, Allan W. Nielsen wrote:
> The 07/29/2019 15:50, Nikolay Aleksandrov wrote:
>> On 29/07/2019 15:22, Nikolay Aleksandrov wrote:
>>> Hi Allan,
>>> On 29/07/2019 15:14, Allan W. Nielsen wrote:
>>>> First of all, as mentioned further down in this thread, I realized that our
>>>> implementation of the multicast floodmasks does not align with the existing SW
>>>> implementation. We will change this, such that all multicast packets goes to the
>>>> SW bridge.
>>>>
>>>> This changes things a bit, not that much.
>>>>
>>>> I actually think you summarized the issue we have (after changing to multicast
>>>> flood-masks) right here:
>>>>
>>>> The 07/26/2019 12:26, Nikolay Aleksandrov wrote:
>>>>>>> Actually you mentioned non-IP traffic, so the querier stuff is not a problem. This
>>>>>>> traffic will always be flooded by the bridge (and also a copy will be locally sent up).
>>>>>>> Thus only the flooding may need to be controlled.
>>>>
>>>> This seems to be exactly what we need.
>>>>
>>>> Assuming we have a SW bridge (br0) with 4 slave interfaces (eth0-3). We use this
>>>> on a network where we want to limit the flooding of frames with dmac
>>>> 01:21:6C:00:00:01 (which is non IP traffic) to eth0 and eth1.
>>>>
>>>> One way of doing this could potentially be to support the following command:
>>>>
>>>> bridge fdb add    01:21:6C:00:00:01 port eth0
>>>> bridge fdb append 01:21:6C:00:00:01 port eth1
>>>>
>>
>> And the fdbs become linked lists?
> Yes, it will most likely become a linked list
> 
>> So we'll increase the complexity for something that is already supported by
>> ACLs (e.g. tc) and also bridge per-port multicast flood flag ?
> I do not think it can be supported with the facilities we have today in tc.
> 
> We can do half of it (copy more fraems to the CPU) with tc, but we can not limit
> the floodmask of a frame with tc (say we want it to flood to 2 out of 4 slave
> ports).
> 

Why not ? You attach an egress filter for the ports and allow that dmac on only
2 of the ports.

>> I'm sorry but that doesn't sound good to me for a case which is very rare and
>> there are existing ways to solve without incurring performance hits or increasing
>> code complexity.
> I do not consider it rarely, controling the forwarding of L2 multicast frames is
> quite common in the applications we are doing.
> 
>> If you find a way to achieve this without incurring a performance hit or significant
>> code complexity increase, and without breaking current use-cases (e.g. unexpected default
>> forwarding behaviour changes) then please send a patch and we can discuss it further with
>> all details present. People have provided enough alternatives which avoid all of the
>> problems.
> Will do, thanks for the guidance.
> 
> /Allan
> 


^ permalink raw reply

* [PATCH v4 11/14] NFC: nxp-nci: Remove unused macro pr_fmt()
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
  To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
	Sedat Dilek
  Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>

The macro had never been used.

The driver uses mostly the nfc_err(), which, with other macros in the family,
is backed by corresponding dev_err(). pr_fmt() is not used for dev_err()
macro. Moreover, there is no need to print the module name which is part of the
device instance name anyway.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 drivers/nfc/nxp-nci/i2c.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 59b0a02a813d..307bd2afbe05 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -12,8 +12,6 @@
  * Copyright (C) 2012  Intel Corporation. All rights reserved.
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
 #include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
-- 
2.20.1


^ permalink raw reply related

* [PATCH v4 08/14] NFC: nxp-nci: Constify acpi_device_id
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
  To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
	Sedat Dilek
  Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>

The content of acpi_device_id is not supposed to change at runtime.
All functions working with acpi_device_id provided by <linux/acpi.h>
work with const acpi_device_id. So mark the non-const structs as const.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 drivers/nfc/nxp-nci/i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index bec9b1ea78e2..4e71962dc557 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -330,7 +330,7 @@ static const struct of_device_id of_nxp_nci_i2c_match[] = {
 MODULE_DEVICE_TABLE(of, of_nxp_nci_i2c_match);
 
 #ifdef CONFIG_ACPI
-static struct acpi_device_id acpi_id[] = {
+static const struct acpi_device_id acpi_id[] = {
 	{ "NXP1001" },
 	{ "NXP7471" },
 	{ },
-- 
2.20.1


^ permalink raw reply related

* [PATCH v4 14/14] NFC: nxp-nci: Fix recommendation for NFC_NXP_NCI_I2C Kconfig
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
  To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
	Sedat Dilek
  Cc: Andy Shevchenko, Oleg Zhurakivskyy
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>

From: Sedat Dilek <sedat.dilek@credativ.de>

This is a simple cleanup to the Kconfig help text as discussed in [1].

[1] https://marc.info/?t=155774435600001&r=1&w=2

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Suggested-by: Oleg Zhurakivskyy <oleg.zhurakivskyy@intel.com>
Signed-off-by: Sedat Dilek <sedat.dilek@credativ.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/nfc/nxp-nci/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/nxp-nci/Kconfig b/drivers/nfc/nxp-nci/Kconfig
index 746b91aa74f0..e1f71deab6fc 100644
--- a/drivers/nfc/nxp-nci/Kconfig
+++ b/drivers/nfc/nxp-nci/Kconfig
@@ -22,4 +22,4 @@ config NFC_NXP_NCI_I2C
 
 	  To compile this driver as a module, choose m here. The module will
 	  be called nxp_nci_i2c.
-	  Say Y if unsure.
+	  Say N if unsure.
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Allan W. Nielsen @ 2019-07-29 14:35 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: Horatiu Vultur, roopa, davem, bridge, netdev, linux-kernel
In-Reply-To: <e4cd0db9-695a-82a7-7dc0-623ded66a4e5@cumulusnetworks.com>

The 07/29/2019 17:21, Nikolay Aleksandrov wrote:
> On 29/07/2019 16:52, Allan W. Nielsen wrote:
> > The 07/29/2019 15:50, Nikolay Aleksandrov wrote:
> >> On 29/07/2019 15:22, Nikolay Aleksandrov wrote:
> >>> Hi Allan,
> >>> On 29/07/2019 15:14, Allan W. Nielsen wrote:
> >>>> First of all, as mentioned further down in this thread, I realized that our
> >>>> implementation of the multicast floodmasks does not align with the existing SW
> >>>> implementation. We will change this, such that all multicast packets goes to the
> >>>> SW bridge.
> >>>>
> >>>> This changes things a bit, not that much.
> >>>>
> >>>> I actually think you summarized the issue we have (after changing to multicast
> >>>> flood-masks) right here:
> >>>>
> >>>> The 07/26/2019 12:26, Nikolay Aleksandrov wrote:
> >>>>>>> Actually you mentioned non-IP traffic, so the querier stuff is not a problem. This
> >>>>>>> traffic will always be flooded by the bridge (and also a copy will be locally sent up).
> >>>>>>> Thus only the flooding may need to be controlled.
> >>>>
> >>>> This seems to be exactly what we need.
> >>>>
> >>>> Assuming we have a SW bridge (br0) with 4 slave interfaces (eth0-3). We use this
> >>>> on a network where we want to limit the flooding of frames with dmac
> >>>> 01:21:6C:00:00:01 (which is non IP traffic) to eth0 and eth1.
> >>>>
> >>>> One way of doing this could potentially be to support the following command:
> >>>>
> >>>> bridge fdb add    01:21:6C:00:00:01 port eth0
> >>>> bridge fdb append 01:21:6C:00:00:01 port eth1
> >> And the fdbs become linked lists?
> > Yes, it will most likely become a linked list
> > 
> >> So we'll increase the complexity for something that is already supported by
> >> ACLs (e.g. tc) and also bridge per-port multicast flood flag ?
> > I do not think it can be supported with the facilities we have today in tc.
> > 
> > We can do half of it (copy more fraems to the CPU) with tc, but we can not limit
> > the floodmask of a frame with tc (say we want it to flood to 2 out of 4 slave
> > ports).
> Why not ? You attach an egress filter for the ports and allow that dmac on only
> 2 of the ports.
Because we want a solution which we eventually can offload in HW. And the HW
facilities we have is doing ingress processing (we have no egress ACLs in this
design), and if we try to offload an egress rule, with an ingress HW facility,
then we will run into other issues.

/Allan

^ permalink raw reply

* [PATCH v4 05/14] NFC: nxp-nci: Add GPIO ACPI mapping table
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
  To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
	Sedat Dilek
  Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>

In order to unify GPIO resource request prepare gpiod_get_index()
to behave correctly when there is no mapping provided by firmware.

Here we add explicit mapping between _CRS GpioIo() resources and
their names used in the driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 drivers/nfc/nxp-nci/i2c.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 713c267acf88..7344405feddf 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -247,6 +247,15 @@ static irqreturn_t nxp_nci_i2c_irq_thread_fn(int irq, void *phy_id)
 	return IRQ_NONE;
 }
 
+static const struct acpi_gpio_params firmware_gpios = { 1, 0, false };
+static const struct acpi_gpio_params enable_gpios = { 2, 0, false };
+
+static const struct acpi_gpio_mapping acpi_nxp_nci_gpios[] = {
+	{ "enable-gpios", &enable_gpios, 1 },
+	{ "firmware-gpios", &firmware_gpios, 1 },
+	{ }
+};
+
 static int nxp_nci_i2c_parse_devtree(struct i2c_client *client)
 {
 	struct nxp_nci_i2c_phy *phy = i2c_get_clientdata(client);
@@ -269,9 +278,14 @@ static int nxp_nci_i2c_parse_devtree(struct i2c_client *client)
 static int nxp_nci_i2c_acpi_config(struct nxp_nci_i2c_phy *phy)
 {
 	struct i2c_client *client = phy->i2c_dev;
+	int r;
 
-	phy->gpiod_en = devm_gpiod_get_index(&client->dev, NULL, 2, GPIOD_OUT_LOW);
-	phy->gpiod_fw = devm_gpiod_get_index(&client->dev, NULL, 1, GPIOD_OUT_LOW);
+	r = devm_acpi_dev_add_driver_gpios(&client->dev, acpi_nxp_nci_gpios);
+	if (r)
+		return r;
+
+	phy->gpiod_en = devm_gpiod_get(&client->dev, "enable", GPIOD_OUT_LOW);
+	phy->gpiod_fw = devm_gpiod_get(&client->dev, "firmware", GPIOD_OUT_LOW);
 
 	if (IS_ERR(phy->gpiod_en) || IS_ERR(phy->gpiod_fw)) {
 		nfc_err(&client->dev, "No GPIOs\n");
-- 
2.20.1


^ permalink raw reply related

* [PATCH v4 07/14] NFC: nxp-nci: Get rid of useless label
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
  To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
	Sedat Dilek
  Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>

Return directly in ->probe() since there no special cleaning is needed.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 drivers/nfc/nxp-nci/i2c.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 6a627d1b6f85..bec9b1ea78e2 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -265,16 +265,13 @@ static int nxp_nci_i2c_probe(struct i2c_client *client,
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
 		nfc_err(&client->dev, "Need I2C_FUNC_I2C\n");
-		r = -ENODEV;
-		goto probe_exit;
+		return -ENODEV;
 	}
 
 	phy = devm_kzalloc(&client->dev, sizeof(struct nxp_nci_i2c_phy),
 			   GFP_KERNEL);
-	if (!phy) {
-		r = -ENOMEM;
-		goto probe_exit;
-	}
+	if (!phy)
+		return -ENOMEM;
 
 	phy->i2c_dev = client;
 	i2c_set_clientdata(client, phy);
@@ -298,7 +295,7 @@ static int nxp_nci_i2c_probe(struct i2c_client *client,
 	r = nxp_nci_probe(phy, &client->dev, &i2c_phy_ops,
 			  NXP_NCI_I2C_MAX_PAYLOAD, &phy->ndev);
 	if (r < 0)
-		goto probe_exit;
+		return r;
 
 	r = request_threaded_irq(client->irq, NULL,
 				 nxp_nci_i2c_irq_thread_fn,
@@ -307,7 +304,6 @@ static int nxp_nci_i2c_probe(struct i2c_client *client,
 	if (r < 0)
 		nfc_err(&client->dev, "Unable to register IRQ handler\n");
 
-probe_exit:
 	return r;
 }
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH] init: Kconfig: consistent indentions
From: Enrico Weigelt, metux IT consult @ 2019-07-29 14:48 UTC (permalink / raw)
  To: linux-kernel; +Cc: kafai, songliubraving, yhs, netdev, bpf, clang-built-linux

From: Enrico Weigelt <info@metux.net>

Just make the indentions consistent with the rest of the file,
as well as most other Kconfig files.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 init/Kconfig | 54 +++++++++++++++++++++++++++---------------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index bd7d650..1a589c6 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -161,13 +161,13 @@ config LOCALVERSION_AUTO
 	  which is done within the script "scripts/setlocalversion".)
 
 config BUILD_SALT
-       string "Build ID Salt"
-       default ""
-       help
-          The build ID is used to link binaries and their debug info. Setting
-          this option will use the value in the calculation of the build id.
-          This is mostly useful for distributions which want to ensure the
-          build is unique between builds. It's safe to leave the default.
+	string "Build ID Salt"
+	default ""
+	help
+	  The build ID is used to link binaries and their debug info. Setting
+	  this option will use the value in the calculation of the build id.
+	  This is mostly useful for distributions which want to ensure the
+	  build is unique between builds. It's safe to leave the default.
 
 config HAVE_KERNEL_GZIP
 	bool
@@ -1294,9 +1294,9 @@ menuconfig EXPERT
 	select DEBUG_KERNEL
 	help
 	  This option allows certain base kernel options and settings
-          to be disabled or tweaked. This is for specialized
-          environments which can tolerate a "non-standard" kernel.
-          Only use this if you really know what you are doing.
+	  to be disabled or tweaked. This is for specialized
+	  environments which can tolerate a "non-standard" kernel.
+	  Only use this if you really know what you are doing.
 
 config UID16
 	bool "Enable 16-bit UID system calls" if EXPERT
@@ -1406,11 +1406,11 @@ config BUG
 	bool "BUG() support" if EXPERT
 	default y
 	help
-          Disabling this option eliminates support for BUG and WARN, reducing
-          the size of your kernel image and potentially quietly ignoring
-          numerous fatal conditions. You should only consider disabling this
-          option for embedded systems with no facilities for reporting errors.
-          Just say Y.
+	  Disabling this option eliminates support for BUG and WARN, reducing
+	  the size of your kernel image and potentially quietly ignoring
+	  numerous fatal conditions. You should only consider disabling this
+	  option for embedded systems with no facilities for reporting errors.
+	  Just say Y.
 
 config ELF_CORE
 	depends on COREDUMP
@@ -1426,8 +1426,8 @@ config PCSPKR_PLATFORM
 	select I8253_LOCK
 	default y
 	help
-          This option allows to disable the internal PC-Speaker
-          support, saving some memory.
+	  This option allows to disable the internal PC-Speaker
+	  support, saving some memory.
 
 config BASE_FULL
 	default y
@@ -1555,18 +1555,18 @@ config KALLSYMS_ALL
 	bool "Include all symbols in kallsyms"
 	depends on DEBUG_KERNEL && KALLSYMS
 	help
-	   Normally kallsyms only contains the symbols of functions for nicer
-	   OOPS messages and backtraces (i.e., symbols from the text and inittext
-	   sections). This is sufficient for most cases. And only in very rare
-	   cases (e.g., when a debugger is used) all symbols are required (e.g.,
-	   names of variables from the data sections, etc).
+	  Normally kallsyms only contains the symbols of functions for nicer
+	  OOPS messages and backtraces (i.e., symbols from the text and inittext
+	  sections). This is sufficient for most cases. And only in very rare
+	  cases (e.g., when a debugger is used) all symbols are required (e.g.,
+	  names of variables from the data sections, etc).
 
-	   This option makes sure that all symbols are loaded into the kernel
-	   image (i.e., symbols from all sections) in cost of increased kernel
-	   size (depending on the kernel configuration, it may be 300KiB or
-	   something like this).
+	  This option makes sure that all symbols are loaded into the kernel
+	  image (i.e., symbols from all sections) in cost of increased kernel
+	  size (depending on the kernel configuration, it may be 300KiB or
+	  something like this).
 
-	   Say N unless you really need all symbols.
+	  Say N unless you really need all symbols.
 
 config KALLSYMS_ABSOLUTE_PERCPU
 	bool
-- 
1.9.1


^ permalink raw reply related

* [PATCH] arcnet: com20020-isa: Mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2019-07-29 14:25 UTC (permalink / raw)
  To: Michael Grzeschik, David S. Miller
  Cc: netdev, linux-kernel, Gustavo A. R. Silva, Geert Uytterhoeven,
	Kees Cook

Mark switch cases where we are expecting to fall through.

This patch fixes the following warnings:

drivers/net/arcnet/com20020-isa.c: warning: this statement may fall
through [-Wimplicit-fallthrough=]:  => 205:13, 203:10, 209:7, 201:11,
207:8

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/arcnet/com20020-isa.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/arcnet/com20020-isa.c b/drivers/net/arcnet/com20020-isa.c
index 28510e33924f..cd27fdc1059b 100644
--- a/drivers/net/arcnet/com20020-isa.c
+++ b/drivers/net/arcnet/com20020-isa.c
@@ -197,16 +197,22 @@ static int __init com20020isa_setup(char *s)
 	switch (ints[0]) {
 	default:		/* ERROR */
 		pr_info("Too many arguments\n");
+		/* Fall through */
 	case 6:		/* Timeout */
 		timeout = ints[6];
+		/* Fall through */
 	case 5:		/* CKP value */
 		clockp = ints[5];
+		/* Fall through */
 	case 4:		/* Backplane flag */
 		backplane = ints[4];
+		/* Fall through */
 	case 3:		/* Node ID */
 		node = ints[3];
+		/* Fall through */
 	case 2:		/* IRQ */
 		irq = ints[2];
+		/* Fall through */
 	case 1:		/* IO address */
 		io = ints[1];
 	}
-- 
2.22.0


^ permalink raw reply related

* [PATCH v4 03/14] NFC: nxp-nci: Get rid of platform data
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
  To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
	Sedat Dilek
  Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>

Legacy platform data must go away. We are on the safe side here since
there are no users of it in the kernel.

If anyone by any odd reason needs it the GPIO lookup tables and
built-in device properties at your service.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 MAINTAINERS                           |  1 -
 drivers/nfc/nxp-nci/core.c            |  1 -
 drivers/nfc/nxp-nci/i2c.c             |  9 +--------
 drivers/nfc/nxp-nci/nxp-nci.h         |  1 -
 include/linux/platform_data/nxp-nci.h | 19 -------------------
 5 files changed, 1 insertion(+), 30 deletions(-)
 delete mode 100644 include/linux/platform_data/nxp-nci.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 99f64e395623..f204f7a65428 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11353,7 +11353,6 @@ F:	include/net/nfc/
 F:	include/uapi/linux/nfc.h
 F:	drivers/nfc/
 F:	include/linux/platform_data/nfcmrvl.h
-F:	include/linux/platform_data/nxp-nci.h
 F:	Documentation/devicetree/bindings/net/nfc/
 
 NFS, SUNRPC, AND LOCKD CLIENTS
diff --git a/drivers/nfc/nxp-nci/core.c b/drivers/nfc/nxp-nci/core.c
index 8dafc696719f..aed18ca60170 100644
--- a/drivers/nfc/nxp-nci/core.c
+++ b/drivers/nfc/nxp-nci/core.c
@@ -14,7 +14,6 @@
 #include <linux/gpio.h>
 #include <linux/module.h>
 #include <linux/nfc.h>
-#include <linux/platform_data/nxp-nci.h>
 
 #include <net/nfc/nci_core.h>
 
diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 5db71869f04b..47b3b7e612e6 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -23,7 +23,6 @@
 #include <linux/gpio/consumer.h>
 #include <linux/of_gpio.h>
 #include <linux/of_irq.h>
-#include <linux/platform_data/nxp-nci.h>
 #include <asm/unaligned.h>
 
 #include <net/nfc/nfc.h>
@@ -304,7 +303,6 @@ static int nxp_nci_i2c_probe(struct i2c_client *client,
 			    const struct i2c_device_id *id)
 {
 	struct nxp_nci_i2c_phy *phy;
-	struct nxp_nci_nfc_platform_data *pdata;
 	int r;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
@@ -323,17 +321,12 @@ static int nxp_nci_i2c_probe(struct i2c_client *client,
 	phy->i2c_dev = client;
 	i2c_set_clientdata(client, phy);
 
-	pdata = client->dev.platform_data;
-
-	if (!pdata && client->dev.of_node) {
+	if (client->dev.of_node) {
 		r = nxp_nci_i2c_parse_devtree(client);
 		if (r < 0) {
 			nfc_err(&client->dev, "Failed to get DT data\n");
 			goto probe_exit;
 		}
-	} else if (pdata) {
-		phy->gpio_en = pdata->gpio_en;
-		phy->gpio_fw = pdata->gpio_fw;
 	} else if (ACPI_HANDLE(&client->dev)) {
 		r = nxp_nci_i2c_acpi_config(phy);
 		if (r < 0)
diff --git a/drivers/nfc/nxp-nci/nxp-nci.h b/drivers/nfc/nxp-nci/nxp-nci.h
index 6fe7c45544bf..ae3fb2735a4e 100644
--- a/drivers/nfc/nxp-nci/nxp-nci.h
+++ b/drivers/nfc/nxp-nci/nxp-nci.h
@@ -14,7 +14,6 @@
 #include <linux/completion.h>
 #include <linux/firmware.h>
 #include <linux/nfc.h>
-#include <linux/platform_data/nxp-nci.h>
 
 #include <net/nfc/nci_core.h>
 
diff --git a/include/linux/platform_data/nxp-nci.h b/include/linux/platform_data/nxp-nci.h
deleted file mode 100644
index 97827ad468e2..000000000000
--- a/include/linux/platform_data/nxp-nci.h
+++ /dev/null
@@ -1,19 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Generic platform data for the NXP NCI NFC chips.
- *
- * Copyright (C) 2014  NXP Semiconductors  All rights reserved.
- *
- * Authors: Clément Perrochaud <clement.perrochaud@nxp.com>
- */
-
-#ifndef _NXP_NCI_H_
-#define _NXP_NCI_H_
-
-struct nxp_nci_nfc_platform_data {
-	unsigned int gpio_en;
-	unsigned int gpio_fw;
-	unsigned int irq;
-};
-
-#endif /* _NXP_NCI_H_ */
-- 
2.20.1


^ permalink raw reply related

* [PATCH v4 01/14] NFC: fix attrs checks in netlink interface
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
  To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
	Sedat Dilek
  Cc: Andrey Konovalov, Andy Shevchenko
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>

From: Andrey Konovalov <andreyknvl@google.com>

nfc_genl_deactivate_target() relies on the NFC_ATTR_TARGET_INDEX
attribute being present, but doesn't check whether it is actually
provided by the user. Same goes for nfc_genl_fw_download() and
NFC_ATTR_FIRMWARE_NAME.

This patch adds appropriate checks.

Found with syzkaller.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 net/nfc/netlink.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/nfc/netlink.c b/net/nfc/netlink.c
index 4a30309bb67f..60fd2748d0ea 100644
--- a/net/nfc/netlink.c
+++ b/net/nfc/netlink.c
@@ -970,7 +970,8 @@ static int nfc_genl_dep_link_down(struct sk_buff *skb, struct genl_info *info)
 	int rc;
 	u32 idx;
 
-	if (!info->attrs[NFC_ATTR_DEVICE_INDEX])
+	if (!info->attrs[NFC_ATTR_DEVICE_INDEX] ||
+	    !info->attrs[NFC_ATTR_TARGET_INDEX])
 		return -EINVAL;
 
 	idx = nla_get_u32(info->attrs[NFC_ATTR_DEVICE_INDEX]);
@@ -1018,7 +1019,8 @@ static int nfc_genl_llc_get_params(struct sk_buff *skb, struct genl_info *info)
 	struct sk_buff *msg = NULL;
 	u32 idx;
 
-	if (!info->attrs[NFC_ATTR_DEVICE_INDEX])
+	if (!info->attrs[NFC_ATTR_DEVICE_INDEX] ||
+	    !info->attrs[NFC_ATTR_FIRMWARE_NAME])
 		return -EINVAL;
 
 	idx = nla_get_u32(info->attrs[NFC_ATTR_DEVICE_INDEX]);
-- 
2.20.1


^ permalink raw reply related

* [PATCH v4 13/14] NFC: nxp-nci: Clarify on supported chips
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
  To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
	Sedat Dilek
  Cc: Andy Shevchenko, Oleg Zhurakivskyy
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>

From: Sedat Dilek <sedat.dilek@credativ.de>

This patch clarifies on the supported NXP NCI chips and families
and lists PN547 and PN548 separately which are known as NPC100
respectively NPC300.

This helps to find informations and identify drivers on vendor's
support websites.

For details see the discussion in [1] and [2].

[1] https://marc.info/?t=155774435600001&r=1&w=2
[2] https://patchwork.kernel.org/project/linux-wireless/list/?submitter=33142

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Suggested-by: Oleg Zhurakivskyy <oleg.zhurakivskyy@intel.com>
Signed-off-by: Sedat Dilek <sedat.dilek@credativ.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Oleg Zhurakivskyy <oleg.zhurakivskyy@intel.com>
---
 drivers/nfc/nxp-nci/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/nxp-nci/Kconfig b/drivers/nfc/nxp-nci/Kconfig
index ed6cbdf0f0b4..746b91aa74f0 100644
--- a/drivers/nfc/nxp-nci/Kconfig
+++ b/drivers/nfc/nxp-nci/Kconfig
@@ -3,8 +3,8 @@ config NFC_NXP_NCI
 	tristate "NXP-NCI NFC driver"
 	depends on NFC_NCI
 	---help---
-	  Generic core driver for NXP NCI chips such as the NPC100
-	  or PN7150 families.
+	  Generic core driver for NXP NCI chips such as the NPC100 (PN547),
+	  NPC300 (PN548) or PN7150 families.
 	  This is a driver based on the NCI NFC kernel layers and
 	  will thus not work with NXP libnfc library.
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH v4 06/14] NFC: nxp-nci: Get rid of code duplication in ->probe()
From: Andy Shevchenko @ 2019-07-29 13:35 UTC (permalink / raw)
  To: Clément Perrochaud, Charles Gorand, netdev, David S. Miller,
	Sedat Dilek
  Cc: Andy Shevchenko, Sedat Dilek
In-Reply-To: <20190729133514.13164-1-andriy.shevchenko@linux.intel.com>

Since OF and ACPI case almost the same get rid of code duplication
by moving gpiod_get() calls directly to ->probe().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 drivers/nfc/nxp-nci/i2c.c | 68 +++++++++------------------------------
 1 file changed, 15 insertions(+), 53 deletions(-)

diff --git a/drivers/nfc/nxp-nci/i2c.c b/drivers/nfc/nxp-nci/i2c.c
index 7344405feddf..6a627d1b6f85 100644
--- a/drivers/nfc/nxp-nci/i2c.c
+++ b/drivers/nfc/nxp-nci/i2c.c
@@ -256,48 +256,10 @@ static const struct acpi_gpio_mapping acpi_nxp_nci_gpios[] = {
 	{ }
 };
 
-static int nxp_nci_i2c_parse_devtree(struct i2c_client *client)
-{
-	struct nxp_nci_i2c_phy *phy = i2c_get_clientdata(client);
-
-	phy->gpiod_en = devm_gpiod_get(&client->dev, "enable", GPIOD_OUT_LOW);
-	if (IS_ERR(phy->gpiod_en)) {
-		nfc_err(&client->dev, "Failed to get EN gpio\n");
-		return PTR_ERR(phy->gpiod_en);
-	}
-
-	phy->gpiod_fw = devm_gpiod_get(&client->dev, "firmware", GPIOD_OUT_LOW);
-	if (IS_ERR(phy->gpiod_fw)) {
-		nfc_err(&client->dev, "Failed to get FW gpio\n");
-		return PTR_ERR(phy->gpiod_fw);
-	}
-
-	return 0;
-}
-
-static int nxp_nci_i2c_acpi_config(struct nxp_nci_i2c_phy *phy)
-{
-	struct i2c_client *client = phy->i2c_dev;
-	int r;
-
-	r = devm_acpi_dev_add_driver_gpios(&client->dev, acpi_nxp_nci_gpios);
-	if (r)
-		return r;
-
-	phy->gpiod_en = devm_gpiod_get(&client->dev, "enable", GPIOD_OUT_LOW);
-	phy->gpiod_fw = devm_gpiod_get(&client->dev, "firmware", GPIOD_OUT_LOW);
-
-	if (IS_ERR(phy->gpiod_en) || IS_ERR(phy->gpiod_fw)) {
-		nfc_err(&client->dev, "No GPIOs\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int nxp_nci_i2c_probe(struct i2c_client *client,
 			    const struct i2c_device_id *id)
 {
+	struct device *dev = &client->dev;
 	struct nxp_nci_i2c_phy *phy;
 	int r;
 
@@ -317,20 +279,20 @@ static int nxp_nci_i2c_probe(struct i2c_client *client,
 	phy->i2c_dev = client;
 	i2c_set_clientdata(client, phy);
 
-	if (client->dev.of_node) {
-		r = nxp_nci_i2c_parse_devtree(client);
-		if (r < 0) {
-			nfc_err(&client->dev, "Failed to get DT data\n");
-			goto probe_exit;
-		}
-	} else if (ACPI_HANDLE(&client->dev)) {
-		r = nxp_nci_i2c_acpi_config(phy);
-		if (r < 0)
-			goto probe_exit;
-	} else {
-		nfc_err(&client->dev, "No platform data\n");
-		r = -EINVAL;
-		goto probe_exit;
+	r = devm_acpi_dev_add_driver_gpios(dev, acpi_nxp_nci_gpios);
+	if (r)
+		return r;
+
+	phy->gpiod_en = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(phy->gpiod_en)) {
+		nfc_err(dev, "Failed to get EN gpio\n");
+		return PTR_ERR(phy->gpiod_en);
+	}
+
+	phy->gpiod_fw = devm_gpiod_get(dev, "firmware", GPIOD_OUT_LOW);
+	if (IS_ERR(phy->gpiod_fw)) {
+		nfc_err(dev, "Failed to get FW gpio\n");
+		return PTR_ERR(phy->gpiod_fw);
 	}
 
 	r = nxp_nci_probe(phy, &client->dev, &i2c_phy_ops,
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v3 11/14] NFC: nxp-nci: Remove unused macro pr_fmt()
From: Sedat Dilek @ 2019-07-29 14:58 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: David Miller, clement.perrochaud, charles.gorand, netdev,
	Sedat Dilek
In-Reply-To: <20190729094047.GH9224@smile.fi.intel.com>

On Mon, Jul 29, 2019 at 12:38 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Jul 26, 2019 at 02:23:46PM -0700, David Miller wrote:
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Date: Thu, 25 Jul 2019 22:35:08 +0300
> >
> > > The macro had never been used.
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> >  ...
> > > @@ -12,8 +12,6 @@
> > >   * Copyright (C) 2012  Intel Corporation. All rights reserved.
> > >   */
> > >
> > > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >
> > If there are any kernel log messages generated, which is the case in
> > this file, this is used.
>
> AFAICS no, it's not.
> All nfc_*() macros are built on top of dev_*() ones for which pr_fmt() is no-op.
> If we would like to have it in that way, we rather should use dev_fmt().
>
>
> > Also, please resubmit this series with a proper header posting containing
> > a high level description of what this patch series does, how it is doing it,
> > and why it is doing it that way.  Also include a changelog.
>
> Will do.
>
> Thank you for review!
>

Can you send out the latest series as v5?
I got some new? patches from you, but a bit confused now.

- Sedat -

^ permalink raw reply

* [PATCH] net/af_iucv: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2019-07-29 14:59 UTC (permalink / raw)
  To: Julian Wiedmann, Ursula Braun, David S. Miller
  Cc: linux-s390, netdev, linux-kernel, Gustavo A. R. Silva,
	Geert Uytterhoeven, Kees Cook

Mark switch cases where we are expecting to fall through.

This patch fixes the following warnings:

net/iucv/af_iucv.c: warning: this statement may fall
through [-Wimplicit-fallthrough=]:  => 537:3, 519:6, 2246:6, 510:6

Notice that, in this particular case, the code comment is
modified in accordance with what GCC is expecting to find.

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 net/iucv/af_iucv.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index 09e1694b6d34..ebb62a4ebe30 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -512,7 +512,9 @@ static void iucv_sock_close(struct sock *sk)
 			sk->sk_state = IUCV_DISCONN;
 			sk->sk_state_change(sk);
 		}
-	case IUCV_DISCONN:   /* fall through */
+		/* fall through */
+
+	case IUCV_DISCONN:
 		sk->sk_state = IUCV_CLOSING;
 		sk->sk_state_change(sk);
 
@@ -525,8 +527,9 @@ static void iucv_sock_close(struct sock *sk)
 					iucv_sock_in_state(sk, IUCV_CLOSED, 0),
 					timeo);
 		}
+		/* fall through */
 
-	case IUCV_CLOSING:   /* fall through */
+	case IUCV_CLOSING:
 		sk->sk_state = IUCV_CLOSED;
 		sk->sk_state_change(sk);
 
@@ -535,8 +538,9 @@ static void iucv_sock_close(struct sock *sk)
 
 		skb_queue_purge(&iucv->send_skb_q);
 		skb_queue_purge(&iucv->backlog_skb_q);
+		/* fall through */
 
-	default:   /* fall through */
+	default:
 		iucv_sever_path(sk, 1);
 	}
 
@@ -2247,10 +2251,10 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
 			kfree_skb(skb);
 			break;
 		}
-		/* fall through and receive non-zero length data */
+		/* fall through - and receive non-zero length data */
 	case (AF_IUCV_FLAG_SHT):
 		/* shutdown request */
-		/* fall through and receive zero length data */
+		/* fall through - and receive zero length data */
 	case 0:
 		/* plain data frame */
 		IUCV_SKB_CB(skb)->class = trans_hdr->iucv_hdr.class;
-- 
2.22.0


^ permalink raw reply related

* Re: DSA Rate Limiting in 88E6390
From: Andrew Lunn @ 2019-07-29 15:01 UTC (permalink / raw)
  To: Benjamin Beckmeyer; +Cc: netdev
In-Reply-To: <5a632696-946d-504b-1077-f7eb6d31ec19@eks-engel.de>

On Mon, Jul 29, 2019 at 12:30:20PM +0200, Benjamin Beckmeyer wrote:
> Hi all,
> is there a possibility to set the rate limiting for the 6390 with linux tools?
> I've seen that the driver will init all to zero, so rate limiting is disabled, 
> but there is no solution for it in the ip tool?
> 
> The only thing I found for rate limiting is the tc tool, but I guess this is 
> only a software solution?

Hi Benjamin

In Linux, we accelerate the software solution by offloading it to the
hardware. So TC is what you need here.
 
> Furthermore, does exist a table or a tutorial which functions DSA supports?
> The background is that we are using the DSDT driver (in future maybe the UMSD
> driver) but we would like to switch to the in kernel DSA entirely. And our
> software is using some of the DSDT functions, so I have to find an 
> alternative to these functions.

The DSA framework supports offloading TC. There was some patches a
while back adding ingress rate limiting to one of the DSA drivers, via
TC. I forget which, and i don't think they have been merged yet. If
you can find the patchset, it should give you a good idea how you can
implement support in the mv88e6xxx driver.

	  Andrew

^ permalink raw reply

* Re: [PATCH v3 11/14] NFC: nxp-nci: Remove unused macro pr_fmt()
From: Sedat Dilek @ 2019-07-29 15:08 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: David Miller, clement.perrochaud, charles.gorand, netdev,
	Sedat Dilek
In-Reply-To: <CA+icZUW5H+9VJvxViYYEDCJ-mLa-xudqYScjZFJ8eA6200YZmg@mail.gmail.com>

On Mon, Jul 29, 2019 at 4:58 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Mon, Jul 29, 2019 at 12:38 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Fri, Jul 26, 2019 at 02:23:46PM -0700, David Miller wrote:
> > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Date: Thu, 25 Jul 2019 22:35:08 +0300
> > >
> > > > The macro had never been used.
> > > >
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
> > >  ...
> > > > @@ -12,8 +12,6 @@
> > > >   * Copyright (C) 2012  Intel Corporation. All rights reserved.
> > > >   */
> > > >
> > > > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > >
> > > If there are any kernel log messages generated, which is the case in
> > > this file, this is used.
> >
> > AFAICS no, it's not.
> > All nfc_*() macros are built on top of dev_*() ones for which pr_fmt() is no-op.
> > If we would like to have it in that way, we rather should use dev_fmt().
> >
> >
> > > Also, please resubmit this series with a proper header posting containing
> > > a high level description of what this patch series does, how it is doing it,
> > > and why it is doing it that way.  Also include a changelog.
> >
> > Will do.
> >
> > Thank you for review!
> >
>
> Can you send out the latest series as v5?
> I got some new? patches from you, but a bit confused now.
>

Thanks for the cover-letter.

- Sedat -

[1] https://marc.info/?l=linux-netdev&m=156440732411578&w=2

^ permalink raw reply

* [PATCH] drivers: net: wireless: rsi: return explicit error values
From: Enrico Weigelt, metux IT consult @ 2019-07-29 15:24 UTC (permalink / raw)
  To: linux-kernel; +Cc: amitkarwar, siva8118, kvalo, linux-wireless, netdev

From: Enrico Weigelt <info@metux.net>

Explicitly return constants instead of variable (and rely on
it to be explicitly initialized), if the value is supposed
to be fixed anyways. Align it with the rest of the driver,
which does it the same way.

Signed-off-by: Enrico Weigelt <info@metux.net>
---
 drivers/net/wireless/rsi/rsi_91x_sdio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
index b42cd50..2a3577d 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
@@ -844,11 +844,11 @@ static int rsi_init_sdio_interface(struct rsi_hw *adapter,
 				   struct sdio_func *pfunction)
 {
 	struct rsi_91x_sdiodev *rsi_91x_dev;
-	int status = -ENOMEM;
+	int status;
 
 	rsi_91x_dev = kzalloc(sizeof(*rsi_91x_dev), GFP_KERNEL);
 	if (!rsi_91x_dev)
-		return status;
+		return -ENOMEM;
 
 	adapter->rsi_dev = rsi_91x_dev;
 
@@ -890,7 +890,7 @@ static int rsi_init_sdio_interface(struct rsi_hw *adapter,
 #ifdef CONFIG_RSI_DEBUGFS
 	adapter->num_debugfs_entries = MAX_DEBUGFS_ENTRIES;
 #endif
-	return status;
+	return 0;
 fail:
 	sdio_disable_func(pfunction);
 	sdio_release_host(pfunction);
-- 
1.9.1


^ permalink raw reply related


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