* Re: [PATCH V4 3/3] can: m_can: add missing message RAM initialization
From: Oliver Hartkopp @ 2014-11-07 11:53 UTC (permalink / raw)
To: Dong Aisheng, linux-can; +Cc: mkl, wg, varkabhadram, netdev, linux-arm-kernel
In-Reply-To: <1415349914-9145-3-git-send-email-b29396@freescale.com>
Thanks!
Hopefully we can manage to push the patch series via can/master ;-)
Regards,
Oliver
On 07.11.2014 09:45, Dong Aisheng wrote:
> The M_CAN message RAM is usually equipped with a parity or ECC functionality.
> But RAM cells suffer a hardware reset and can therefore hold arbitrary
> content at startup - including parity and/or ECC bits.
>
> To prevent the M_CAN controller detecting checksum errors when reading
> potentially uninitialized TX message RAM content to transmit CAN
> frames the TX message RAM has to be written with (any kind of) initial
> data.
>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> ---
> ChangeLog:
> v3-v4:
> * initialize the entire Message RAM in use instead of only 8 bytes
> since this is a valid and needed initialization process.
> Patch title also changed.
> ---
> drivers/net/can/m_can/m_can.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index eee1533..de742d0 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1114,7 +1114,7 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> struct resource *res;
> void __iomem *addr;
> u32 out_val[MRAM_CFG_LEN];
> - int ret;
> + int i, start, end, ret;
>
> /* message ram could be shared */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> @@ -1165,6 +1165,15 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> priv->mcfg[MRAM_TXE].off, priv->mcfg[MRAM_TXE].num,
> priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num);
>
> + /* initialize the entire Message RAM in use to avoid possible
> + * ECC/parity checksum errors when reading an uninitialized buffer
> + */
> + start = priv->mcfg[MRAM_SIDF].off;
> + end = priv->mcfg[MRAM_TXB].off +
> + priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
> + for (i = start; i < end; i += 4)
> + writel(0x0, priv->mram_base + i);
> +
> return 0;
> }
>
>
^ permalink raw reply
* Re: [Xen-devel] BUG in xennet_make_frags with paged skb data
From: Stefan Bader @ 2014-11-07 12:15 UTC (permalink / raw)
To: Eric Dumazet, Zoltan Kiss
Cc: netdev, David S. Miller, Konrad Rzeszutek Wilk, Boris Ostrovsky,
David Vrabel, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <1415359320.13896.105.camel@edumazet-glaptop2.roam.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 948 bytes --]
On 07.11.2014 12:22, Eric Dumazet wrote:
> On Fri, 2014-11-07 at 09:25 +0000, Zoltan Kiss wrote:
>
> Please do not top post.
>
>> Hi,
>>
>> AFAIK in this scenario your skb frag is wrong. The page pointer should
>> point to the original compound page (not a member of it), and offset
>> should be set accordingly.
>> For example, if your compound page is 16K (4 page), then the page
>> pointer should point to the first page, and if the data starts at the
>> 3rd page, then offset should be >8K
>
> This is not accurate.
>
> This BUG_ON() is wrong.
>
> It should instead be :
>
> BUG_ON(len + offset > PAGE_SIZE<<compound_order(compound_head(page)));
would that not have to be
BUG_ON((page-compound_head(page)*PAGE_SIZE)+offset+len >
PAGE_SIZE<<compound_order(compound_head(page)));
since offset is adjusted to start from the tail page in that case.
>
> splice() code can generate such cases.
>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [Xen-devel] BUG in xennet_make_frags with paged skb data
From: Zoltan Kiss @ 2014-11-07 12:21 UTC (permalink / raw)
To: Stefan Bader, Eric Dumazet
Cc: netdev, David S. Miller, Konrad Rzeszutek Wilk, Boris Ostrovsky,
David Vrabel, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <545CB7FF.8080003@canonical.com>
On 07/11/14 12:15, Stefan Bader wrote:
> On 07.11.2014 12:22, Eric Dumazet wrote:
>> On Fri, 2014-11-07 at 09:25 +0000, Zoltan Kiss wrote:
>>
>> Please do not top post.
>>
>>> Hi,
>>>
>>> AFAIK in this scenario your skb frag is wrong. The page pointer should
>>> point to the original compound page (not a member of it), and offset
>>> should be set accordingly.
>>> For example, if your compound page is 16K (4 page), then the page
>>> pointer should point to the first page, and if the data starts at the
>>> 3rd page, then offset should be >8K
>>
>> This is not accurate.
>>
>> This BUG_ON() is wrong.
>>
>> It should instead be :
>>
>> BUG_ON(len + offset > PAGE_SIZE<<compound_order(compound_head(page)));
>
> would that not have to be
>
> BUG_ON((page-compound_head(page)*PAGE_SIZE)+offset+len >
> PAGE_SIZE<<compound_order(compound_head(page)));
There should be a parentheses around "page-compound_head(page)".
>
> since offset is adjusted to start from the tail page in that case.
>>
>> splice() code can generate such cases.
>>
>>
>
>
^ permalink raw reply
* Re: [PATCHv4 1/1] net: fec: fix regression on i.MX28 introduced by rx_copybreak support
From: Fabio Estevam @ 2014-11-07 12:26 UTC (permalink / raw)
To: Lothar Waßmann
Cc: netdev@vger.kernel.org, Fabio Estevam, Frank Li, linux-kernel,
Russell King, David S. Miller,
linux-arm-kernel@lists.infradead.org, Stefan Wahren
In-Reply-To: <1415350967-2238-2-git-send-email-LW@KARO-electronics.de>
On Fri, Nov 7, 2014 at 7:02 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:
> commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> introduced a regression for i.MX28. The swap_buffer() function doing
> the endian conversion of the received data on i.MX28 may access memory
> beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> does not copy those bytes, so that the last bytes of a packet may be
> filled with invalid data after swapping.
> This will likely lead to checksum errors on received packets.
> E.g. when trying to mount an NFS rootfs:
> UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
>
> Do the byte swapping and copying to the new skb in one go if
> necessary.
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
With this patch I am able to NFS mount on mx28 again. Thanks, Lothar!
Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
^ permalink raw reply
* Re: [Xen-devel] BUG in xennet_make_frags with paged skb data
From: Stefan Bader @ 2014-11-07 12:28 UTC (permalink / raw)
To: Zoltan Kiss, Eric Dumazet
Cc: netdev, David S. Miller, Konrad Rzeszutek Wilk, Boris Ostrovsky,
David Vrabel, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <545CB92E.9050106@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]
On 07.11.2014 13:21, Zoltan Kiss wrote:
>
>
> On 07/11/14 12:15, Stefan Bader wrote:
>> On 07.11.2014 12:22, Eric Dumazet wrote:
>>> On Fri, 2014-11-07 at 09:25 +0000, Zoltan Kiss wrote:
>>>
>>> Please do not top post.
>>>
>>>> Hi,
>>>>
>>>> AFAIK in this scenario your skb frag is wrong. The page pointer should
>>>> point to the original compound page (not a member of it), and offset
>>>> should be set accordingly.
>>>> For example, if your compound page is 16K (4 page), then the page
>>>> pointer should point to the first page, and if the data starts at the
>>>> 3rd page, then offset should be >8K
>>>
>>> This is not accurate.
>>>
>>> This BUG_ON() is wrong.
>>>
>>> It should instead be :
>>>
>>> BUG_ON(len + offset > PAGE_SIZE<<compound_order(compound_head(page)));
>>
>> would that not have to be
>>
>> BUG_ON((page-compound_head(page)*PAGE_SIZE)+offset+len >
>> PAGE_SIZE<<compound_order(compound_head(page)));
>
> There should be a parentheses around "page-compound_head(page)".
*sigh* before submitting anything I'll have to make sure I run it past the
compiler at least. I never manager to type the beast without some error.
But you got the drift...
>>
>> since offset is adjusted to start from the tail page in that case.
>>>
>>> splice() code can generate such cases.
>>>
>>>
>>
>>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH 0/4] Replace skb_copy_datagram_const_iovec with iterator version
From: Herbert Xu @ 2014-11-07 13:21 UTC (permalink / raw)
To: David Miller; +Cc: viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki
In-Reply-To: <20141106.221313.1375322315446025123.davem@davemloft.net>
Hi Dave:
This patch series adds the helper skb_copy_datagram_iter, which
is meant to replace both skb_copy_datagram_iovec and its evil
twin skb_copy_datagram_const_iovec.
It then converts tun and macvtap over to the new helper and finally
removes skb_copy_datagram_const_iovec which is only used by tun
and macvtap.
The copy_to_iter return value issue pointed out by Al has now been
fixed.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [REGRESSION] in 3.18-rc1: ppp crashes kernel
From: Takashi Iwai @ 2014-11-07 13:22 UTC (permalink / raw)
To: Stefan Seyfried; +Cc: Paul Mackerras, linux-ppp, netdev, LKML
In-Reply-To: <545CA8B6.2060608@message-id.googlemail.com>
At Fri, 07 Nov 2014 12:10:46 +0100,
Stefan Seyfried wrote:
>
> Hi all,
>
> since 3.18-rc1, setting up a PPP interface kills my kernel with
>
> [ 163.433251] PPP generic driver version 2.4.2
> [ 164.452474] ------------[ cut here ]------------
> [ 164.453327] kernel BUG at ../mm/vmalloc.c:1316!
> [ 164.453327] invalid opcode: 0000 [#1] PREEMPT SMP
> [ 164.453327] Modules linked in: ppp_async crc_ccitt ppp_generic slhc af_packet xfs libcrc32c coretemp kvm_intel
> snd_hda_codec_conexant iTCO_wdt snd_hda_codec_generic iTCO_vendor_support uvcvideo snd_hda_intel snd_hda_controller mac80211 videobuf2_vmalloc snd_hda_codec kvm e1000e videobuf2_memops cfg80211 videobuf2_core v4l2_common snd_hwdep i2c_i801 videodev snd_pcm pcspkr thinkpad_acpi serio_raw wmi lpc_ich snd_timer thermal snd rfkill mfd_core tpm_tis shpchp mei_me soundcore ptp mei pps_core acpi_cpufreq tpm battery processor ac dm_mod btrfs xor raid6_pq i915 i2c_algo_bit drm_kms_helper drm video button sg
> [ 164.453327] CPU: 0 PID: 6927 Comm: pppd Not tainted 3.18.0-rc3-3.ge706e91-desktop #1
> [ 164.453327] Hardware name: LENOVO 7470E36/7470E36, BIOS 6DET61WW (3.11 ) 11/10/2009
>
> This is easy to reproduce with:
>
> linux:~ # cat bin/crashme.sh
> ----
> #!/bin/bash -x
> pppd local pty "netcat -l 1234" &
> sleep 1
> pppd local pty "netcat localhost 1234" &
> sleep 1
> ----
>
> 3.17 works fine.
> I bisected the issue multiple times and always arrived at
>
> # first bad commit: [d6dd50e07c5bec00db2005969b1a01f8ca3d25ef] Merge branch 'core-rcu-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
>
> which is a merge commit unfortunately.
>
> The BUG encountered above is in:
>
> 1309 static struct vm_struct *__get_vm_area_node(unsigned long size,
> 1310 unsigned long align, unsigned long flags, unsigned long start,
> 1311 unsigned long end, int node, gfp_t gfp_mask, const void *caller)
> 1312 {
> 1313 struct vmap_area *va;
> 1314 struct vm_struct *area;
> 1315
> 1316 BUG_ON(in_interrupt());
> 1317 if (flags & VM_IOREMAP)
> 1318 align = 1ul << clamp(fls(size), PAGE_SHIFT, IOREMAP_MAX_ORDER);
> 1319
>
> the call trace is:
> [ 164.453327] Call Trace:
> [ 164.453327] [<ffffffff811974bd>] __vmalloc_node_range+0x6d/0x290
> [ 164.453327] [<ffffffff8119771e>] __vmalloc+0x3e/0x50
> [ 164.453327] [<ffffffff81146950>] bpf_prog_alloc+0x30/0xa0
> [ 164.453327] [<ffffffff8157b716>] bpf_prog_create+0x46/0xb0
> [ 164.453327] [<ffffffffa07ecb90>] ppp_ioctl+0x420/0xe9a [ppp_generic]
> [ 164.453327] [<ffffffff811df1c7>] do_vfs_ioctl+0x2e7/0x4c0
> [ 164.453327] [<ffffffff811df421>] SyS_ioctl+0x81/0xa0
> [ 164.453327] [<ffffffff8165ee2d>] system_call_fastpath+0x16/0x1b
> [ 164.453327] [<00007f4502d87397>] 0x7f4502d87397
bpf_prog_create() is called inside spin_lock_bh(), and the BUG_ON()
hits. Below is a quick fix.
Takashi
-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] net: ppp: Don't call bpf_prog_create() in ppp_lock
In ppp_ioctl(), bpf_prog_create() is called inside ppp_lock, which
eventually calls vmalloc() and hits BUG_ON() in vmalloc.c. This patch
works around the problem by moving the allocation outside the lock.
Reported-by: Stefan Seyfried <stefan.seyfried@googlemail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/net/ppp/ppp_generic.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 68c3a3f4e0ab..794a47329368 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -755,23 +755,23 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
err = get_filter(argp, &code);
if (err >= 0) {
+ struct bpf_prog *pass_filter = NULL;
struct sock_fprog_kern fprog = {
.len = err,
.filter = code,
};
- ppp_lock(ppp);
- if (ppp->pass_filter) {
- bpf_prog_destroy(ppp->pass_filter);
- ppp->pass_filter = NULL;
+ err = 0;
+ if (fprog.filter)
+ err = bpf_prog_create(&pass_filter, &fprog);
+ if (!err) {
+ ppp_lock(ppp);
+ if (ppp->pass_filter)
+ bpf_prog_destroy(ppp->pass_filter);
+ ppp->pass_filter = pass_filter;
+ ppp_unlock(ppp);
}
- if (fprog.filter != NULL)
- err = bpf_prog_create(&ppp->pass_filter,
- &fprog);
- else
- err = 0;
kfree(code);
- ppp_unlock(ppp);
}
break;
}
@@ -781,23 +781,23 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
err = get_filter(argp, &code);
if (err >= 0) {
+ struct bpf_prog *active_filter = NULL;
struct sock_fprog_kern fprog = {
.len = err,
.filter = code,
};
- ppp_lock(ppp);
- if (ppp->active_filter) {
- bpf_prog_destroy(ppp->active_filter);
- ppp->active_filter = NULL;
+ err = 0;
+ if (fprog.filter)
+ err = bpf_prog_create(&active_filter, &fprog);
+ if (!err) {
+ ppp_lock(ppp);
+ if (ppp->active_filter)
+ bpf_prog_destroy(ppp->active_filter);
+ ppp->active_filter = active_filter;
+ ppp_unlock(ppp);
}
- if (fprog.filter != NULL)
- err = bpf_prog_create(&ppp->active_filter,
- &fprog);
- else
- err = 0;
kfree(code);
- ppp_unlock(ppp);
}
break;
}
--
2.1.3
^ permalink raw reply related
* [PATCH 1/4] inet: Add skb_copy_datagram_iter
From: Herbert Xu @ 2014-11-07 13:22 UTC (permalink / raw)
To: David Miller, viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki
In-Reply-To: <20141107132135.GA20022@gondor.apana.org.au>
This patch adds skb_copy_datagram_iter, which is identical to
skb_copy_datagram_iovec except that it operates on iov_iter
instead of iovec.
Eventually all users of skb_copy_datagram_iovec should switch
over to iov_iter and then we can remove skb_copy_datagram_iovec.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
include/linux/skbuff.h | 3 +
net/core/datagram.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 53f4f6c..933cfce 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -150,6 +150,7 @@
struct net_device;
struct scatterlist;
struct pipe_inode_info;
+struct iov_iter;
#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
struct nf_conntrack {
@@ -2653,6 +2654,8 @@ int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *frm,
int skb_copy_datagram_const_iovec(const struct sk_buff *from, int offset,
const struct iovec *to, int to_offset,
int size);
+int skb_copy_datagram_iter(const struct sk_buff *from, int offset,
+ struct iov_iter *to, int size);
void skb_free_datagram(struct sock *sk, struct sk_buff *skb);
void skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb);
int skb_kill_datagram(struct sock *sk, struct sk_buff *skb, unsigned int flags);
diff --git a/net/core/datagram.c b/net/core/datagram.c
index fdbc9a8..84d90d0 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -49,6 +49,7 @@
#include <linux/spinlock.h>
#include <linux/slab.h>
#include <linux/pagemap.h>
+#include <linux/uio.h>
#include <net/protocol.h>
#include <linux/skbuff.h>
@@ -482,6 +483,92 @@ fault:
EXPORT_SYMBOL(skb_copy_datagram_const_iovec);
/**
+ * skb_copy_datagram_iter - Copy a datagram to an iovec iterator.
+ * @skb: buffer to copy
+ * @offset: offset in the buffer to start copying from
+ * @to: iovec iterator to copy to
+ * @len: amount of data to copy from buffer to iovec
+ */
+int skb_copy_datagram_iter(const struct sk_buff *skb, int offset,
+ struct iov_iter *to, int len)
+{
+ int start = skb_headlen(skb);
+ int i, copy = start - offset;
+ struct sk_buff *frag_iter;
+
+ trace_skb_copy_datagram_iovec(skb, len);
+
+ /* Copy header. */
+ if (copy > 0) {
+ if (copy > len)
+ copy = len;
+ if (copy_to_iter(skb->data + offset, copy, to) != copy)
+ goto short_copy;
+ if ((len -= copy) == 0)
+ return 0;
+ offset += copy;
+ }
+
+ /* Copy paged appendix. Hmm... why does this look so complicated? */
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ int end;
+ const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+
+ WARN_ON(start > offset + len);
+
+ end = start + skb_frag_size(frag);
+ if ((copy = end - offset) > 0) {
+ if (copy > len)
+ copy = len;
+ if (copy_page_to_iter(skb_frag_page(frag),
+ frag->page_offset + offset -
+ start, copy, to) != copy)
+ goto short_copy;
+ if (!(len -= copy))
+ return 0;
+ offset += copy;
+ }
+ start = end;
+ }
+
+ skb_walk_frags(skb, frag_iter) {
+ int end;
+
+ WARN_ON(start > offset + len);
+
+ end = start + frag_iter->len;
+ if ((copy = end - offset) > 0) {
+ if (copy > len)
+ copy = len;
+ if (skb_copy_datagram_iter(frag_iter, offset - start,
+ to, copy))
+ goto fault;
+ if ((len -= copy) == 0)
+ return 0;
+ offset += copy;
+ }
+ start = end;
+ }
+ if (!len)
+ return 0;
+
+ /* This is not really a user copy fault, but rather someone
+ * gave us a bogus length on the skb. We should probably
+ * print a warning here as it may indicate a kernel bug.
+ */
+
+fault:
+ return -EFAULT;
+
+short_copy:
+ if (iov_iter_count(to))
+ goto fault;
+
+ return 0;
+}
+EXPORT_SYMBOL(skb_copy_datagram_iter);
+
+/**
* skb_copy_datagram_from_iovec - Copy a datagram from an iovec.
* @skb: buffer to copy
* @offset: offset in the buffer to start copying to
^ permalink raw reply related
* [PATCH 2/4] tun: Use iovec iterators
From: Herbert Xu @ 2014-11-07 13:22 UTC (permalink / raw)
To: David Miller, viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki
In-Reply-To: <20141107132135.GA20022@gondor.apana.org.au>
This patch removes the use of skb_copy_datagram_const_iovec in
favour of the iovec iterator-based skb_copy_datagram_iter.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
drivers/net/tun.c | 65 ++++++++++++++++++++++++------------------------------
1 file changed, 30 insertions(+), 35 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 9dd3746..2ff769b 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -71,6 +71,7 @@
#include <net/rtnetlink.h>
#include <net/sock.h>
#include <linux/seq_file.h>
+#include <linux/uio.h>
#include <asm/uaccess.h>
@@ -1230,11 +1231,11 @@ static ssize_t tun_chr_aio_write(struct kiocb *iocb, const struct iovec *iv,
static ssize_t tun_put_user(struct tun_struct *tun,
struct tun_file *tfile,
struct sk_buff *skb,
- const struct iovec *iv, int len)
+ struct iov_iter *iter)
{
struct tun_pi pi = { 0, skb->protocol };
- ssize_t total = 0;
- int vlan_offset = 0, copied;
+ ssize_t total;
+ int vlan_offset;
int vlan_hlen = 0;
int vnet_hdr_sz = 0;
@@ -1244,23 +1245,25 @@ static ssize_t tun_put_user(struct tun_struct *tun,
if (tun->flags & TUN_VNET_HDR)
vnet_hdr_sz = tun->vnet_hdr_sz;
+ total = skb->len + vlan_hlen + vnet_hdr_sz;
+
if (!(tun->flags & TUN_NO_PI)) {
- if ((len -= sizeof(pi)) < 0)
+ if (iov_iter_count(iter) < sizeof(pi))
return -EINVAL;
- if (len < skb->len + vlan_hlen + vnet_hdr_sz) {
+ total += sizeof(pi);
+ if (iov_iter_count(iter) < total) {
/* Packet will be striped */
pi.flags |= TUN_PKT_STRIP;
}
- if (memcpy_toiovecend(iv, (void *) &pi, 0, sizeof(pi)))
+ if (copy_to_iter(&pi, sizeof(pi), iter) != sizeof(pi))
return -EFAULT;
- total += sizeof(pi);
}
if (vnet_hdr_sz) {
struct virtio_net_hdr gso = { 0 }; /* no info leak */
- if ((len -= vnet_hdr_sz) < 0)
+ if (iov_iter_count(iter) < vnet_hdr_sz)
return -EINVAL;
if (skb_is_gso(skb)) {
@@ -1299,17 +1302,12 @@ static ssize_t tun_put_user(struct tun_struct *tun,
gso.flags = VIRTIO_NET_HDR_F_DATA_VALID;
} /* else everything is zero */
- if (unlikely(memcpy_toiovecend(iv, (void *)&gso, total,
- sizeof(gso))))
+ if (copy_to_iter(&gso, sizeof(gso), iter) != sizeof(gso))
return -EFAULT;
- total += vnet_hdr_sz;
}
- copied = total;
- len = min_t(int, skb->len + vlan_hlen, len);
- total += skb->len + vlan_hlen;
if (vlan_hlen) {
- int copy, ret;
+ int ret;
struct {
__be16 h_vlan_proto;
__be16 h_vlan_TCI;
@@ -1320,36 +1318,32 @@ static ssize_t tun_put_user(struct tun_struct *tun,
vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto);
- copy = min_t(int, vlan_offset, len);
- ret = skb_copy_datagram_const_iovec(skb, 0, iv, copied, copy);
- len -= copy;
- copied += copy;
- if (ret || !len)
+ ret = skb_copy_datagram_iter(skb, 0, iter, vlan_offset);
+ if (ret || !iov_iter_count(iter))
goto done;
- copy = min_t(int, sizeof(veth), len);
- ret = memcpy_toiovecend(iv, (void *)&veth, copied, copy);
- len -= copy;
- copied += copy;
- if (ret || !len)
+ ret = copy_to_iter(&veth, sizeof(veth), iter);
+ if (ret != sizeof(veth) || !iov_iter_count(iter))
goto done;
}
- skb_copy_datagram_const_iovec(skb, vlan_offset, iv, copied, len);
+ skb_copy_datagram_iter(skb, vlan_offset, iter, skb->len - vlan_offset);
done:
tun->dev->stats.tx_packets++;
- tun->dev->stats.tx_bytes += len;
+ tun->dev->stats.tx_bytes += skb->len + vlan_hlen;
return total;
}
static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
- const struct iovec *iv, ssize_t len, int noblock)
+ const struct iovec *iv, unsigned long segs,
+ ssize_t len, int noblock)
{
struct sk_buff *skb;
ssize_t ret = 0;
int peeked, err, off = 0;
+ struct iov_iter iter;
tun_debug(KERN_INFO, tun, "tun_do_read\n");
@@ -1362,11 +1356,12 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
/* Read frames from queue */
skb = __skb_recv_datagram(tfile->socket.sk, noblock ? MSG_DONTWAIT : 0,
&peeked, &off, &err);
- if (skb) {
- ret = tun_put_user(tun, tfile, skb, iv, len);
- kfree_skb(skb);
- } else
- ret = err;
+ if (!skb)
+ return ret;
+
+ iov_iter_init(&iter, READ, iv, segs, len);
+ ret = tun_put_user(tun, tfile, skb, &iter);
+ kfree_skb(skb);
return ret;
}
@@ -1387,7 +1382,7 @@ static ssize_t tun_chr_aio_read(struct kiocb *iocb, const struct iovec *iv,
goto out;
}
- ret = tun_do_read(tun, tfile, iv, len,
+ ret = tun_do_read(tun, tfile, iv, count, len,
file->f_flags & O_NONBLOCK);
ret = min_t(ssize_t, ret, len);
if (ret > 0)
@@ -1488,7 +1483,7 @@ static int tun_recvmsg(struct kiocb *iocb, struct socket *sock,
SOL_PACKET, TUN_TX_TIMESTAMP);
goto out;
}
- ret = tun_do_read(tun, tfile, m->msg_iov, total_len,
+ ret = tun_do_read(tun, tfile, m->msg_iov, m->msg_iovlen, total_len,
flags & MSG_DONTWAIT);
if (ret > total_len) {
m->msg_flags |= MSG_TRUNC;
^ permalink raw reply related
* [PATCH 3/4] macvtap: Use iovec iterators
From: Herbert Xu @ 2014-11-07 13:22 UTC (permalink / raw)
To: David Miller, viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki
In-Reply-To: <20141107132135.GA20022@gondor.apana.org.au>
This patch removes the use of skb_copy_datagram_const_iovec in
favour of the iovec iterator-based skb_copy_datagram_iter.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
drivers/net/macvtap.c | 46 +++++++++++++++++++++-------------------------
1 file changed, 21 insertions(+), 25 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 880cc09..cea99d4 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -15,6 +15,7 @@
#include <linux/cdev.h>
#include <linux/idr.h>
#include <linux/fs.h>
+#include <linux/uio.h>
#include <net/ipv6.h>
#include <net/net_namespace.h>
@@ -778,31 +779,29 @@ static ssize_t macvtap_aio_write(struct kiocb *iocb, const struct iovec *iv,
/* Put packet to the user space buffer */
static ssize_t macvtap_put_user(struct macvtap_queue *q,
const struct sk_buff *skb,
- const struct iovec *iv, int len)
+ struct iov_iter *iter)
{
int ret;
int vnet_hdr_len = 0;
int vlan_offset = 0;
- int copied, total;
+ int total;
if (q->flags & IFF_VNET_HDR) {
struct virtio_net_hdr vnet_hdr;
vnet_hdr_len = q->vnet_hdr_sz;
- if ((len -= vnet_hdr_len) < 0)
+ if (iov_iter_count(iter) < vnet_hdr_len)
return -EINVAL;
macvtap_skb_to_vnet_hdr(skb, &vnet_hdr);
- if (memcpy_toiovecend(iv, (void *)&vnet_hdr, 0, sizeof(vnet_hdr)))
+ if (copy_to_iter(&vnet_hdr, sizeof(vnet_hdr), iter) !=
+ sizeof(vnet_hdr))
return -EFAULT;
}
- total = copied = vnet_hdr_len;
+ total = vnet_hdr_len;
total += skb->len;
- if (!vlan_tx_tag_present(skb))
- len = min_t(int, skb->len, len);
- else {
- int copy;
+ if (vlan_tx_tag_present(skb)) {
struct {
__be16 h_vlan_proto;
__be16 h_vlan_TCI;
@@ -811,37 +810,33 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
veth.h_vlan_TCI = htons(vlan_tx_tag_get(skb));
vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto);
- len = min_t(int, skb->len + VLAN_HLEN, len);
total += VLAN_HLEN;
- copy = min_t(int, vlan_offset, len);
- ret = skb_copy_datagram_const_iovec(skb, 0, iv, copied, copy);
- len -= copy;
- copied += copy;
- if (ret || !len)
+ ret = skb_copy_datagram_iter(skb, 0, iter, vlan_offset);
+ if (ret || !iov_iter_count(iter))
goto done;
- copy = min_t(int, sizeof(veth), len);
- ret = memcpy_toiovecend(iv, (void *)&veth, copied, copy);
- len -= copy;
- copied += copy;
- if (ret || !len)
+ ret = copy_to_iter(&veth, sizeof(veth), iter);
+ if (ret != sizeof(veth) || !iov_iter_count(iter))
goto done;
}
- ret = skb_copy_datagram_const_iovec(skb, vlan_offset, iv, copied, len);
+ ret = skb_copy_datagram_iter(skb, vlan_offset, iter,
+ skb->len - vlan_offset);
done:
return ret ? ret : total;
}
static ssize_t macvtap_do_read(struct macvtap_queue *q,
- const struct iovec *iv, unsigned long len,
+ const struct iovec *iv, unsigned long segs,
+ unsigned long len,
int noblock)
{
DEFINE_WAIT(wait);
struct sk_buff *skb;
ssize_t ret = 0;
+ struct iov_iter iter;
while (len) {
if (!noblock)
@@ -863,7 +858,8 @@ static ssize_t macvtap_do_read(struct macvtap_queue *q,
schedule();
continue;
}
- ret = macvtap_put_user(q, skb, iv, len);
+ iov_iter_init(&iter, READ, iv, segs, len);
+ ret = macvtap_put_user(q, skb, &iter);
kfree_skb(skb);
break;
}
@@ -886,7 +882,7 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
goto out;
}
- ret = macvtap_do_read(q, iv, len, file->f_flags & O_NONBLOCK);
+ ret = macvtap_do_read(q, iv, count, len, file->f_flags & O_NONBLOCK);
ret = min_t(ssize_t, ret, len);
if (ret > 0)
iocb->ki_pos = ret;
@@ -1117,7 +1113,7 @@ static int macvtap_recvmsg(struct kiocb *iocb, struct socket *sock,
int ret;
if (flags & ~(MSG_DONTWAIT|MSG_TRUNC))
return -EINVAL;
- ret = macvtap_do_read(q, m->msg_iov, total_len,
+ ret = macvtap_do_read(q, m->msg_iov, m->msg_iovlen, total_len,
flags & MSG_DONTWAIT);
if (ret > total_len) {
m->msg_flags |= MSG_TRUNC;
^ permalink raw reply related
* [PATCH 4/4] net: Kill skb_copy_datagram_const_iovec
From: Herbert Xu @ 2014-11-07 13:22 UTC (permalink / raw)
To: David Miller, viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki
In-Reply-To: <20141107132135.GA20022@gondor.apana.org.au>
Now that both macvtap and tun are using skb_copy_datagram_iter, we
can kill the abomination that is skb_copy_datagram_const_iovec.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
include/linux/skbuff.h | 3 -
net/core/datagram.c | 89 -------------------------------------------------
2 files changed, 92 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 933cfce..103fbe8 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2651,9 +2651,6 @@ int skb_copy_datagram_from_iovec(struct sk_buff *skb, int offset,
int len);
int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *frm,
int offset, size_t count);
-int skb_copy_datagram_const_iovec(const struct sk_buff *from, int offset,
- const struct iovec *to, int to_offset,
- int size);
int skb_copy_datagram_iter(const struct sk_buff *from, int offset,
struct iov_iter *to, int size);
void skb_free_datagram(struct sock *sk, struct sk_buff *skb);
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 84d90d0..26391a3 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -394,95 +394,6 @@ fault:
EXPORT_SYMBOL(skb_copy_datagram_iovec);
/**
- * skb_copy_datagram_const_iovec - Copy a datagram to an iovec.
- * @skb: buffer to copy
- * @offset: offset in the buffer to start copying from
- * @to: io vector to copy to
- * @to_offset: offset in the io vector to start copying to
- * @len: amount of data to copy from buffer to iovec
- *
- * Returns 0 or -EFAULT.
- * Note: the iovec is not modified during the copy.
- */
-int skb_copy_datagram_const_iovec(const struct sk_buff *skb, int offset,
- const struct iovec *to, int to_offset,
- int len)
-{
- int start = skb_headlen(skb);
- int i, copy = start - offset;
- struct sk_buff *frag_iter;
-
- /* Copy header. */
- if (copy > 0) {
- if (copy > len)
- copy = len;
- if (memcpy_toiovecend(to, skb->data + offset, to_offset, copy))
- goto fault;
- if ((len -= copy) == 0)
- return 0;
- offset += copy;
- to_offset += copy;
- }
-
- /* Copy paged appendix. Hmm... why does this look so complicated? */
- for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
- int end;
- const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
-
- WARN_ON(start > offset + len);
-
- end = start + skb_frag_size(frag);
- if ((copy = end - offset) > 0) {
- int err;
- u8 *vaddr;
- struct page *page = skb_frag_page(frag);
-
- if (copy > len)
- copy = len;
- vaddr = kmap(page);
- err = memcpy_toiovecend(to, vaddr + frag->page_offset +
- offset - start, to_offset, copy);
- kunmap(page);
- if (err)
- goto fault;
- if (!(len -= copy))
- return 0;
- offset += copy;
- to_offset += copy;
- }
- start = end;
- }
-
- skb_walk_frags(skb, frag_iter) {
- int end;
-
- WARN_ON(start > offset + len);
-
- end = start + frag_iter->len;
- if ((copy = end - offset) > 0) {
- if (copy > len)
- copy = len;
- if (skb_copy_datagram_const_iovec(frag_iter,
- offset - start,
- to, to_offset,
- copy))
- goto fault;
- if ((len -= copy) == 0)
- return 0;
- offset += copy;
- to_offset += copy;
- }
- start = end;
- }
- if (!len)
- return 0;
-
-fault:
- return -EFAULT;
-}
-EXPORT_SYMBOL(skb_copy_datagram_const_iovec);
-
-/**
* skb_copy_datagram_iter - Copy a datagram to an iovec iterator.
* @skb: buffer to copy
* @offset: offset in the buffer to start copying from
^ permalink raw reply related
* [PATCH] usbnet: smsc95xx: dereferencing NULL pointer
From: Sudip Mukherjee @ 2014-11-07 13:22 UTC (permalink / raw)
To: Steve Glendinning; +Cc: Sudip Mukherjee, netdev, linux-usb, linux-kernel
we were dereferencing dev to initialize pdata. but just after that we
have a BUG_ON(!dev). so we were basically dereferencing the pointer
first and then tesing it for NULL.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/net/usb/smsc95xx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index d07bf4c..3393238 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1670,12 +1670,13 @@ done:
static int smsc95xx_resume(struct usb_interface *intf)
{
struct usbnet *dev = usb_get_intfdata(intf);
- struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ struct smsc95xx_priv *pdata;
u8 suspend_flags = pdata->suspend_flags;
int ret;
u32 val;
BUG_ON(!dev);
+ pdata = (struct smsc95xx_priv *)(dev->data[0]);
netdev_dbg(dev->net, "resume suspend_flags=0x%02x\n", suspend_flags);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 0/2] ipv4: Simplify raw_probe_proto_opt and avoid reading user iov twice
From: Herbert Xu @ 2014-11-07 13:25 UTC (permalink / raw)
To: David Miller; +Cc: viro, netdev, linux-kernel, bcrl, nakam, yoshfuji
In-Reply-To: <20141107020025.GC9101@gondor.apana.org.au>
Hi Dave:
This series rewrites the function raw_probe_proto_opt in a more
readable fasion, and then fixes the long-standing bug where we
read the probed bytes twice which means that what we're using to
probe may in fact be invalid.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH 1/2] ipv4: Use standard iovec primitive in raw_probe_proto_opt
From: Herbert Xu @ 2014-11-07 13:27 UTC (permalink / raw)
To: David Miller, viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki,
nakam
In-Reply-To: <20141107132553.GA20190@gondor.apana.org.au>
The function raw_probe_proto_opt tries to extract the first two
bytes from the user input in order to seed the IPsec lookup for
ICMP packets. In doing so it's processing iovec by hand and
overcomplicating things.
This patch replaces the manual iovec processing with a call to
memcpy_fromiovecend.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/ipv4/raw.c | 50 +++++++++++---------------------------------------
1 file changed, 11 insertions(+), 39 deletions(-)
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index ee8fa4b..9be9050 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -422,48 +422,20 @@ error:
static int raw_probe_proto_opt(struct flowi4 *fl4, struct msghdr *msg)
{
- struct iovec *iov;
- u8 __user *type = NULL;
- u8 __user *code = NULL;
- int probed = 0;
- unsigned int i;
+ struct icmphdr icmph;
+ int err;
- if (!msg->msg_iov)
+ if (fl4->flowi4_proto != IPPROTO_ICMP)
return 0;
- for (i = 0; i < msg->msg_iovlen; i++) {
- iov = &msg->msg_iov[i];
- if (!iov)
- continue;
-
- switch (fl4->flowi4_proto) {
- case IPPROTO_ICMP:
- /* check if one-byte field is readable or not. */
- if (iov->iov_base && iov->iov_len < 1)
- break;
-
- if (!type) {
- type = iov->iov_base;
- /* check if code field is readable or not. */
- if (iov->iov_len > 1)
- code = type + 1;
- } else if (!code)
- code = iov->iov_base;
-
- if (type && code) {
- if (get_user(fl4->fl4_icmp_type, type) ||
- get_user(fl4->fl4_icmp_code, code))
- return -EFAULT;
- probed = 1;
- }
- break;
- default:
- probed = 1;
- break;
- }
- if (probed)
- break;
- }
+ /* We only need the first two bytes. */
+ err = memcpy_fromiovecend((void *)&icmph, msg->msg_iov, 0, 2);
+ if (err)
+ return err;
+
+ fl4->fl4_icmp_type = icmph.type;
+ fl4->fl4_icmp_code = icmph.code;
+
return 0;
}
^ permalink raw reply related
* [PATCH 2/2] ipv4: Avoid reading user iov twice after raw_probe_proto_opt
From: Herbert Xu @ 2014-11-07 13:27 UTC (permalink / raw)
To: David Miller, viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki,
nakam
In-Reply-To: <20141107132553.GA20190@gondor.apana.org.au>
Ever since raw_probe_proto_opt was added it had the problem of
causing the user iov to be read twice, once during the probe for
the protocol header and once again in ip_append_data.
This is a potential security problem since it means that whatever
we're probing may be invalid. This patch plugs the hole by
firstly advancing the iov so we don't read the same spot again,
and secondly saving what we read the first time around for use
by ip_append_data.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/ipv4/raw.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 54 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 9be9050..43385a9 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -79,6 +79,16 @@
#include <linux/netfilter.h>
#include <linux/netfilter_ipv4.h>
#include <linux/compat.h>
+#include <linux/uio.h>
+
+struct raw_frag_vec {
+ struct iovec *iov;
+ union {
+ struct icmphdr icmph;
+ char c[1];
+ } hdr;
+ int hlen;
+};
static struct raw_hashinfo raw_v4_hashinfo = {
.lock = __RW_LOCK_UNLOCKED(raw_v4_hashinfo.lock),
@@ -420,25 +430,57 @@ error:
return err;
}
-static int raw_probe_proto_opt(struct flowi4 *fl4, struct msghdr *msg)
+static int raw_probe_proto_opt(struct raw_frag_vec *rfv, struct flowi4 *fl4)
{
- struct icmphdr icmph;
int err;
if (fl4->flowi4_proto != IPPROTO_ICMP)
return 0;
/* We only need the first two bytes. */
- err = memcpy_fromiovecend((void *)&icmph, msg->msg_iov, 0, 2);
+ rfv->hlen = 2;
+
+ err = memcpy_fromiovec(rfv->hdr.c, rfv->iov, rfv->hlen);
if (err)
return err;
- fl4->fl4_icmp_type = icmph.type;
- fl4->fl4_icmp_code = icmph.code;
+ fl4->fl4_icmp_type = rfv->hdr.icmph.type;
+ fl4->fl4_icmp_code = rfv->hdr.icmph.code;
return 0;
}
+static int raw_getfrag(void *from, char *to, int offset, int len, int odd,
+ struct sk_buff *skb)
+{
+ struct raw_frag_vec *rfv = from;
+
+ if (offset < rfv->hlen) {
+ int copy = min(rfv->hlen - offset, len);
+
+ if (skb->ip_summed == CHECKSUM_PARTIAL)
+ memcpy(to, rfv->hdr.c + offset, copy);
+ else
+ skb->csum = csum_block_add(
+ skb->csum,
+ csum_partial_copy_nocheck(rfv->hdr.c + offset,
+ to, copy, 0),
+ odd);
+
+ odd = 0;
+ offset += copy;
+ to += copy;
+ len -= copy;
+
+ if (!len)
+ return 0;
+ }
+
+ offset -= rfv->hlen;
+
+ return ip_generic_getfrag(rfv->iov, to, offset, len, odd, skb);
+}
+
static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
size_t len)
{
@@ -452,6 +494,7 @@ static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
u8 tos;
int err;
struct ip_options_data opt_copy;
+ struct raw_frag_vec rfv;
err = -EMSGSIZE;
if (len > 0xFFFF)
@@ -557,7 +600,10 @@ static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
daddr, saddr, 0, 0);
if (!inet->hdrincl) {
- err = raw_probe_proto_opt(&fl4, msg);
+ rfv.iov = msg->msg_iov;
+ rfv.hlen = 0;
+
+ err = raw_probe_proto_opt(&rfv, &fl4);
if (err)
goto done;
}
@@ -588,8 +634,8 @@ back_from_confirm:
if (!ipc.addr)
ipc.addr = fl4.daddr;
lock_sock(sk);
- err = ip_append_data(sk, &fl4, ip_generic_getfrag,
- msg->msg_iov, len, 0,
+ err = ip_append_data(sk, &fl4, raw_getfrag,
+ &rfv, len, 0,
&ipc, &rt, msg->msg_flags);
if (err)
ip_flush_pending_frames(sk);
^ permalink raw reply related
* Re: [net-next 1/9] i40e: poll firmware slower
From: Or Gerlitz @ 2014-11-07 13:29 UTC (permalink / raw)
To: Jeff Kirsher
Cc: David Miller, Kamil Krawczyk, Linux Netdev List, nhorman,
sassmann, jogreene
In-Reply-To: <1415350670-15333-2-git-send-email-jeffrey.t.kirsher@intel.com>
On Fri, Nov 7, 2014 at 10:57 AM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> The code was polling the firmware tail register for completion
any reason not to sleep while waiting for this completion? can the
firmware generate an interrupt?
^ permalink raw reply
* Re: [net-next 6/9] ixgbe: fix X540 Completion timeout
From: Sergei Shtylyov @ 2014-11-07 14:35 UTC (permalink / raw)
To: Jeff Kirsher, davem; +Cc: Don Skidmore, netdev, nhorman, sassmann, jogreene
In-Reply-To: <1415350670-15333-7-git-send-email-jeffrey.t.kirsher@intel.com>
Hello.
On 11/7/2014 11:57 AM, Jeff Kirsher wrote:
> From: Don Skidmore <donald.c.skidmore@intel.com>
> On topologies including few levels of PCIe switching X540 can run into an
> unexpected completion error. We get around this by waiting after enabling
> loopback a sufficient amount of time until Tx Data Fetch is sent. We then
> poll the pending transaction bit to ensure we received the completion. Only
> then do we go on to clear the buffers.
> Signed-of-by: Don Skidmore <donald.c.skidmore@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> index b5f484b..e314b53 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
[...]
> @@ -3600,6 +3601,24 @@ void ixgbe_clear_tx_pending(struct ixgbe_hw *hw)
> hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0);
> IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg0 | IXGBE_HLREG0_LPBK);
>
> + /* wait for a last completion before clearing buffers */
> + IXGBE_WRITE_FLUSH(hw);
> + usleep_range(3000, 6000);
> +
> + /* Before proceeding, make sure that the PCIe block does not have
> + * transactions pending.
> + */
> + poll = ixgbe_pcie_timeout_poll(hw);
> + for (i = 0; i < poll; i++) {
> + usleep_range(100, 200);
> + value = ixgbe_read_pci_cfg_word(hw, IXGBE_PCI_DEVICE_STATUS);
> + if (ixgbe_removed(hw->hw_addr))
> + goto out;
Why not just *break*?
> + if (!(value & IXGBE_PCI_DEVICE_STATUS_TRANSACTION_PENDING))
> + goto out;
Likewise.
> + }
> +
> +out:
> /* initiate cleaning flow for buffers in the PCIe transaction layer */
> gcr_ext = IXGBE_READ_REG(hw, IXGBE_GCR_EXT);
> IXGBE_WRITE_REG(hw, IXGBE_GCR_EXT,
WBR, Sergei
^ permalink raw reply
* [PATCH] stmmac: platform: fix sparse warnings
From: Andy Shevchenko @ 2014-11-07 14:46 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, David S . Miller, Vince Bridgers
Cc: Andy Shevchenko
This patch fixes the following sparse warnings. One is fixed by casting return
value to a return type of the function. The others by creating a specific
stmmac_platform.h which provides the bits related to the platform driver.
drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c:59:29: warning: incorrect type in return expression (different address spaces)
drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c:59:29: expected void *
drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c:59:29: got void [noderef] <asn:2>*reg
drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c:64:29: warning: symbol 'meson6_dwmac_data' was not declared. Should it be static?
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c:354:29: warning: symbol 'stih4xx_dwmac_data' was not declared. Should it be static?
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c:361:29: warning: symbol 'stid127_dwmac_data' was not declared. Should it be static?
drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c:133:29: warning: symbol 'sun7i_gmac_data' was not declared. Should it be static?
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c | 4 +++-
.../net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 2 ++
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 2 ++
drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c | 2 ++
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 5 ----
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 ++
.../net/ethernet/stmicro/stmmac/stmmac_platform.h | 28 ++++++++++++++++++++++
7 files changed, 39 insertions(+), 6 deletions(-)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c
index d225a60..cca028d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c
@@ -18,6 +18,8 @@
#include <linux/platform_device.h>
#include <linux/stmmac.h>
+#include "stmmac_platform.h"
+
#define ETHMAC_SPEED_100 BIT(1)
struct meson_dwmac {
@@ -56,7 +58,7 @@ static void *meson6_dwmac_setup(struct platform_device *pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
dwmac->reg = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(dwmac->reg))
- return dwmac->reg;
+ return ERR_CAST(dwmac->reg);
return dwmac;
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 3aad413..e97074c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -23,7 +23,9 @@
#include <linux/regmap.h>
#include <linux/reset.h>
#include <linux/stmmac.h>
+
#include "stmmac.h"
+#include "stmmac_platform.h"
#define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII 0x0
#define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_RGMII 0x1
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
index ccfe7e5..ea40692 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
@@ -22,6 +22,8 @@
#include <linux/of.h>
#include <linux/of_net.h>
+#include "stmmac_platform.h"
+
#define DWMAC_125MHZ 125000000
#define DWMAC_50MHZ 50000000
#define DWMAC_25MHZ 25000000
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c
index 771cd15..a26bda2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c
@@ -22,6 +22,8 @@
#include <linux/of_net.h>
#include <linux/regulator/consumer.h>
+#include "stmmac_platform.h"
+
struct sunxi_priv_data {
int interface;
int clk_enabled;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 709798b..bd75ee8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -135,11 +135,6 @@ void stmmac_disable_eee_mode(struct stmmac_priv *priv);
bool stmmac_eee_init(struct stmmac_priv *priv);
#ifdef CONFIG_STMMAC_PLATFORM
-extern const struct stmmac_of_data meson6_dwmac_data;
-extern const struct stmmac_of_data sun7i_gmac_data;
-extern const struct stmmac_of_data stih4xx_dwmac_data;
-extern const struct stmmac_of_data stid127_dwmac_data;
-extern const struct stmmac_of_data socfpga_gmac_data;
extern struct platform_driver stmmac_pltfr_driver;
static inline int stmmac_register_platform(void)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index f4fe854..9f18401 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -27,7 +27,9 @@
#include <linux/of.h>
#include <linux/of_net.h>
#include <linux/of_device.h>
+
#include "stmmac.h"
+#include "stmmac_platform.h"
static const struct of_device_id stmmac_dt_ids[] = {
/* SoC specific glue layers should come before generic bindings */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
new file mode 100644
index 0000000..25dd1f7
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
@@ -0,0 +1,28 @@
+/*******************************************************************************
+ Copyright (C) 2007-2009 STMicroelectronics Ltd
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms and conditions of the GNU General Public License,
+ version 2, as published by the Free Software Foundation.
+
+ This program is distributed in the hope it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ The full GNU General Public License is included in this distribution in
+ the file called "COPYING".
+
+ Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+*******************************************************************************/
+
+#ifndef __STMMAC_PLATFORM_H__
+#define __STMMAC_PLATFORM_H__
+
+extern const struct stmmac_of_data meson6_dwmac_data;
+extern const struct stmmac_of_data sun7i_gmac_data;
+extern const struct stmmac_of_data stih4xx_dwmac_data;
+extern const struct stmmac_of_data stid127_dwmac_data;
+extern const struct stmmac_of_data socfpga_gmac_data;
+
+#endif /* __STMMAC_PLATFORM_H__ */
--
2.1.3
^ permalink raw reply related
* [PATCH v4 0/8] net: can: Use syscon regmap for TI specific RAMINIT register
From: Roger Quadros @ 2014-11-07 14:49 UTC (permalink / raw)
To: wg, mkl
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros
Hi,
Some hardware (TI am43xx) has a buggy RAMINIT DONE mechanism and it might
not always set the DONE bit. This will result in a lockup in c_can_hw_raminit_wait_ti(),
so patch 1 adds a timeout mechanism there.
There is a non compliancy within TI platforms with respect to the
layout of the RAMINIT register. The patches 3 and 4 address this issue
and make a flexible but standard way of defining the RAMINIT hardware register
layout in the device tree. The RAMINIT register is accessed using the syscon
regmap framework.
Patches available at
git@github.com:rogerq/linux.git [for-v3.19/can]
Patches are tested on am335x-evm, am437x-gp-evm and dra7-evm.
Board support files to allow CAN testing on these boards are available at
git@github.com:rogerq/linux.git [for-v3.19/omap-dts-dcan]
Changelog:
v4:
- updated "syscon-raminit" binding to contain the CAN instance id. We no longer
use different compatible ids for multiple CAN IPs on the same SoC. The instance
ID is used instead to locate the start/stop bit positions from driver data.
v3:
- allow driver data to be more than just CAN_ID
- RAMINIT register data moved to driver data instead of device tree file.
v2:
- added "ti" vendor prefix to TI specific raminit properties.
- split DTS changes into a separate series
cheers,
-roger
---
Roger Quadros (8):
net: can: c_can: Add timeout to c_can_hw_raminit_ti()
net: can: c_can: Introduce c_can_driver_data structure
net: can: c_can: Add RAMINIT register information to driver data
net: can: c_can: Add syscon/regmap RAMINIT mechanism
net: can: c_can: Add support for START pulse in RAMINIT sequence
net: can: c_can: Disable pins when CAN interface is down
net: can: c_can: Add support for TI DRA7 DCAN
net: can: c_can: Add support for TI am3352 DCAN
.../devicetree/bindings/net/can/c_can.txt | 4 +
drivers/net/can/c_can/c_can.c | 20 ++
drivers/net/can/c_can/c_can.h | 23 ++-
drivers/net/can/c_can/c_can_platform.c | 217 +++++++++++++++------
4 files changed, 203 insertions(+), 61 deletions(-)
--
1.8.3.2
^ permalink raw reply
* [PATCH v4 1/8] net: can: c_can: Add timeout to c_can_hw_raminit_ti()
From: Roger Quadros @ 2014-11-07 14:49 UTC (permalink / raw)
To: wg, mkl
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros
In-Reply-To: <1415371762-29885-1-git-send-email-rogerq@ti.com>
TI's RAMINIT DONE mechanism is buggy on AM43xx SoC and may not always
be set after the START bit is set. Although it seems to work fine even
in that case. So add a timeout mechanism to c_can_hw_raminit_wait_ti().
Don't bail out in that failure case but just print an error message.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/net/can/c_can/c_can_platform.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index fb279d6..b144e71 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -75,10 +75,18 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv,
static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask,
u32 val)
{
+ int timeout = 0;
/* We look only at the bits of our instance. */
val &= mask;
- while ((readl(priv->raminit_ctrlreg) & mask) != val)
+ while ((readl(priv->raminit_ctrlreg) & mask) != val) {
udelay(1);
+ timeout++;
+
+ if (timeout == 1000) {
+ dev_err(&priv->dev->dev, "%s: time out\n", __func__);
+ break;
+ }
+ }
}
static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
--
1.8.3.2
^ permalink raw reply related
* [PATCH v4 3/8] net: can: c_can: Add RAMINIT register information to driver data
From: Roger Quadros @ 2014-11-07 14:49 UTC (permalink / raw)
To: wg, mkl
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros
In-Reply-To: <1415371762-29885-1-git-send-email-rogerq@ti.com>
Some platforms (e.g. TI) need special RAMINIT register handling.
Provide a way to store RAMINIT register description in driver data.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/net/can/c_can/c_can.h | 6 ++++++
drivers/net/can/c_can/c_can_platform.c | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 26c975d..3c305a1 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -171,6 +171,12 @@ enum c_can_dev_id {
struct c_can_driver_data {
enum c_can_dev_id id;
+
+ /* RAMINIT register description. Optional. */
+ u8 num_can; /* Number of CAN instances on the SoC */
+ u8 *raminit_start_bits; /* Array of START bit positions */
+ u8 *raminit_done_bits; /* Array of DONE bit positions */
+ bool raminit_pulse; /* If set, sets and clears START bit (pulse) */
};
/* c_can private data structure */
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 1546c2b..20deb67 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -250,6 +250,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
}
priv = netdev_priv(dev);
+
switch (drvdata->id) {
case BOSCH_C_CAN:
priv->regs = reg_map_c_can;
--
1.8.3.2
^ permalink raw reply related
* [PATCH v4 4/8] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Roger Quadros @ 2014-11-07 14:49 UTC (permalink / raw)
To: wg, mkl
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros
In-Reply-To: <1415371762-29885-1-git-send-email-rogerq@ti.com>
Some TI SoCs like DRA7 have a RAMINIT register specification
different from the other AMxx SoCs and as expected by the
existing driver.
To add more insanity, this register is shared with other
IPs like DSS, PCIe and PWM.
Provides a more generic mechanism to specify the RAMINIT
register location and START/DONE bit position and use the
syscon/regmap framework to access the register.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
.../devicetree/bindings/net/can/c_can.txt | 3 +
drivers/net/can/c_can/c_can.h | 11 +-
drivers/net/can/c_can/c_can_platform.c | 112 ++++++++++++++-------
3 files changed, 86 insertions(+), 40 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
index 8f1ae81..a3ca3ee 100644
--- a/Documentation/devicetree/bindings/net/can/c_can.txt
+++ b/Documentation/devicetree/bindings/net/can/c_can.txt
@@ -12,6 +12,9 @@ Required properties:
Optional properties:
- ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the
instance number
+- syscon-raminit : Handle to system control region that contains the
+ RAMINIT register, register offset to the RAMINIT
+ register and the CAN instance number (0 offset).
Note: "ti,hwmods" field is used to fetch the base address and irq
resources from TI, omap hwmod data base during device registration.
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 3c305a1..0e17c7b 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -179,6 +179,14 @@ struct c_can_driver_data {
bool raminit_pulse; /* If set, sets and clears START bit (pulse) */
};
+/* Out of band RAMINIT register access via syscon regmap */
+struct c_can_raminit {
+ struct regmap *syscon; /* for raminit ctrl. reg. access */
+ unsigned int reg; /* register index within syscon */
+ u8 start_bit;
+ u8 done_bit;
+};
+
/* c_can private data structure */
struct c_can_priv {
struct can_priv can; /* must be the first member */
@@ -196,8 +204,7 @@ struct c_can_priv {
const u16 *regs;
void *priv; /* for board-specific data */
enum c_can_dev_id type;
- u32 __iomem *raminit_ctrlreg;
- int instance;
+ struct c_can_raminit raminit_sys; /* RAMINIT via syscon regmap */
void (*raminit) (const struct c_can_priv *priv, bool enable);
u32 comm_rcv_high;
u32 rxmasked;
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 20deb67..3776483 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -32,14 +32,13 @@
#include <linux/clk.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
#include <linux/can/dev.h>
#include "c_can.h"
-#define CAN_RAMINIT_START_MASK(i) (0x001 << (i))
-#define CAN_RAMINIT_DONE_MASK(i) (0x100 << (i))
-#define CAN_RAMINIT_ALL_MASK(i) (0x101 << (i))
#define DCAN_RAM_INIT_BIT (1 << 3)
static DEFINE_SPINLOCK(raminit_lock);
/*
@@ -72,47 +71,61 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv,
writew(val, priv->base + 2 * priv->regs[index]);
}
-static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask,
- u32 val)
+static void c_can_hw_raminit_wait_syscon(const struct c_can_priv *priv,
+ u32 mask, u32 val)
{
int timeout = 0;
+ const struct c_can_raminit *raminit = &priv->raminit_sys;
+ u32 ctrl;
+
/* We look only at the bits of our instance. */
val &= mask;
- while ((readl(priv->raminit_ctrlreg) & mask) != val) {
+ do {
udelay(1);
timeout++;
+ regmap_read(raminit->syscon, raminit->reg, &ctrl);
if (timeout == 1000) {
dev_err(&priv->dev->dev, "%s: time out\n", __func__);
break;
}
- }
+ } while ((ctrl & mask) != val);
}
-static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
+static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable)
{
- u32 mask = CAN_RAMINIT_ALL_MASK(priv->instance);
+ u32 mask;
u32 ctrl;
+ const struct c_can_raminit *raminit = &priv->raminit_sys;
+ u8 start_bit, done_bit;
+
+ start_bit = raminit->start_bit;
+ done_bit = raminit->done_bit;
spin_lock(&raminit_lock);
- ctrl = readl(priv->raminit_ctrlreg);
+ mask = 1 << start_bit | 1 << done_bit;
+ regmap_read(raminit->syscon, raminit->reg, &ctrl);
+
/* We clear the done and start bit first. The start bit is
* looking at the 0 -> transition, but is not self clearing;
* And we clear the init done bit as well.
+ * NOTE: DONE must be written with 1 to clear it.
*/
- ctrl &= ~CAN_RAMINIT_START_MASK(priv->instance);
- ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
- writel(ctrl, priv->raminit_ctrlreg);
- ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance);
- c_can_hw_raminit_wait_ti(priv, mask, ctrl);
+ ctrl &= ~(1 << start_bit);
+ ctrl |= 1 << done_bit;
+ regmap_write(raminit->syscon, raminit->reg, ctrl);
+
+ ctrl &= ~(1 << done_bit);
+ c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
if (enable) {
/* Set start bit and wait for the done bit. */
- ctrl |= CAN_RAMINIT_START_MASK(priv->instance);
- writel(ctrl, priv->raminit_ctrlreg);
- ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
- c_can_hw_raminit_wait_ti(priv, mask, ctrl);
+ ctrl |= 1 << start_bit;
+ regmap_write(raminit->syscon, raminit->reg, ctrl);
+
+ ctrl |= 1 << done_bit;
+ c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
}
spin_unlock(&raminit_lock);
}
@@ -206,10 +219,11 @@ static int c_can_plat_probe(struct platform_device *pdev)
struct net_device *dev;
struct c_can_priv *priv;
const struct of_device_id *match;
- struct resource *mem, *res;
+ struct resource *mem;
int irq;
struct clk *clk;
const struct c_can_driver_data *drvdata;
+ struct device_node *np = pdev->dev.of_node;
match = of_match_device(c_can_of_table, &pdev->dev);
if (match) {
@@ -278,27 +292,49 @@ static int c_can_plat_probe(struct platform_device *pdev)
priv->read_reg32 = d_can_plat_read_reg32;
priv->write_reg32 = d_can_plat_write_reg32;
- if (pdev->dev.of_node)
- priv->instance = of_alias_get_id(pdev->dev.of_node, "d_can");
- else
- priv->instance = pdev->id;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- /* Not all D_CAN modules have a separate register for the D_CAN
- * RAM initialization. Use default RAM init bit in D_CAN module
- * if not specified in DT.
+ /* Check if we need custom RAMINIT via syscon. Mostly for TI
+ * platforms. Only supported with DT boot.
*/
- if (!res) {
+ if (np && of_property_read_bool(np, "syscon-raminit")) {
+ u32 id;
+ struct c_can_raminit *raminit = &priv->raminit_sys;
+
+ ret = -EINVAL;
+ raminit->syscon = syscon_regmap_lookup_by_phandle(np,
+ "syscon-raminit");
+ if (IS_ERR(raminit->syscon)) {
+ dev_err(&pdev->dev,
+ "couldn't get syscon regmap for RAMINIT\n");
+ goto exit_free_device;
+ }
+
+ if (of_property_read_u32_index(np, "syscon-raminit", 1,
+ &raminit->reg)) {
+ dev_err(&pdev->dev,
+ "couldn't get the RAMINIT reg. offset!\n");
+ goto exit_free_device;
+ }
+
+ if (of_property_read_u32_index(np, "syscon-raminit", 2,
+ &id)) {
+ dev_err(&pdev->dev,
+ "couldn't get the CAN instance ID\n");
+ goto exit_free_device;
+ }
+
+ if (id >= drvdata->num_can) {
+ dev_err(&pdev->dev,
+ "Invalid CAN instance ID\n");
+ goto exit_free_device;
+ }
+
+ raminit->start_bit = drvdata->raminit_start_bits[id];
+ raminit->done_bit = drvdata->raminit_done_bits[id];
+
+ priv->raminit = c_can_hw_raminit_syscon;
+ } else {
priv->raminit = c_can_hw_raminit;
- break;
}
-
- priv->raminit_ctrlreg = devm_ioremap(&pdev->dev, res->start,
- resource_size(res));
- if (!priv->raminit_ctrlreg || priv->instance < 0)
- dev_info(&pdev->dev, "control memory is not used for raminit\n");
- else
- priv->raminit = c_can_hw_raminit_ti;
break;
default:
ret = -EINVAL;
--
1.8.3.2
^ permalink raw reply related
* [PATCH v4 6/8] net: can: c_can: Disable pins when CAN interface is down
From: Roger Quadros @ 2014-11-07 14:49 UTC (permalink / raw)
To: wg, mkl
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros
In-Reply-To: <1415371762-29885-1-git-send-email-rogerq@ti.com>
DRA7 CAN IP suffers from a problem which causes it to be prevented
from fully turning OFF (i.e. stuck in transition) if the module was
disabled while there was traffic on the CAN_RX line.
To work around this issue we select the SLEEP pin state by default
on probe and use the DEFAULT pin state on CAN up and back to the
SLEEP pin state on CAN down.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/net/can/c_can/c_can.c | 20 ++++++++++++++++++++
drivers/net/can/c_can/c_can.h | 1 +
drivers/net/can/c_can/c_can_platform.c | 20 ++++++++++++++++++++
3 files changed, 41 insertions(+)
diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 8e78bb4..4dfc3ce 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -35,6 +35,7 @@
#include <linux/list.h>
#include <linux/io.h>
#include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
#include <linux/can.h>
#include <linux/can/dev.h>
@@ -603,6 +604,15 @@ static int c_can_start(struct net_device *dev)
priv->can.state = CAN_STATE_ERROR_ACTIVE;
+ /* activate pins */
+ if (!IS_ERR(priv->pinctrl)) {
+ struct pinctrl_state *s;
+
+ s = pinctrl_lookup_state(priv->pinctrl, PINCTRL_STATE_DEFAULT);
+ if (!IS_ERR(s))
+ pinctrl_select_state(priv->pinctrl, s);
+ }
+
return 0;
}
@@ -611,6 +621,16 @@ static void c_can_stop(struct net_device *dev)
struct c_can_priv *priv = netdev_priv(dev);
c_can_irq_control(priv, false);
+
+ /* deactivate pins */
+ if (!IS_ERR(priv->pinctrl)) {
+ struct pinctrl_state *s;
+
+ s = pinctrl_lookup_state(priv->pinctrl, PINCTRL_STATE_SLEEP);
+ if (!IS_ERR(s))
+ pinctrl_select_state(priv->pinctrl, s);
+ }
+
priv->can.state = CAN_STATE_STOPPED;
}
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index c6715ca..3cedf48 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -210,6 +210,7 @@ struct c_can_priv {
u32 comm_rcv_high;
u32 rxmasked;
u32 dlc[C_CAN_MSG_OBJ_TX_NUM];
+ struct pinctrl *pinctrl;
};
struct net_device *alloc_c_can_dev(void);
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index b838c6b..71b9063 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -34,6 +34,7 @@
#include <linux/of_device.h>
#include <linux/mfd/syscon.h>
#include <linux/regmap.h>
+#include <linux/pinctrl/consumer.h>
#include <linux/can/dev.h>
@@ -230,6 +231,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
struct clk *clk;
const struct c_can_driver_data *drvdata;
struct device_node *np = pdev->dev.of_node;
+ struct pinctrl *pinctrl;
match = of_match_device(c_can_of_table, &pdev->dev);
if (match) {
@@ -241,6 +243,23 @@ static int c_can_plat_probe(struct platform_device *pdev)
return -ENODEV;
}
+ pinctrl = devm_pinctrl_get(&pdev->dev);
+ if (!IS_ERR(pinctrl)) {
+ struct pinctrl_state *s;
+
+ /* Deactivate pins to prevent DRA7 DCAN IP from being
+ * stuck in transition when module is disabled.
+ * Pins are activated in c_can_start() and deactivated
+ * in c_can_stop()
+ */
+ s = pinctrl_lookup_state(pinctrl, PINCTRL_STATE_SLEEP);
+ if (!IS_ERR(s))
+ pinctrl_select_state(pinctrl, s);
+ } else {
+ dev_warn(&pdev->dev,
+ "failed to get pinctrl\n");
+ }
+
/* get the appropriate clk */
clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(clk)) {
@@ -270,6 +289,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
}
priv = netdev_priv(dev);
+ priv->pinctrl = pinctrl;
switch (drvdata->id) {
case BOSCH_C_CAN:
--
1.8.3.2
^ permalink raw reply related
* [PATCH v4 2/8] net: can: c_can: Introduce c_can_driver_data structure
From: Roger Quadros @ 2014-11-07 14:49 UTC (permalink / raw)
To: wg, mkl
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros
In-Reply-To: <1415371762-29885-1-git-send-email-rogerq@ti.com>
We want to have more data than just can_dev_id to be present
in the driver data e.g. TI platforms need RAMINIT register
description. Introduce the c_can_driver_data structure and move
the can_dev_id into it.
Tidy up the way it is used on probe().
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/net/can/c_can/c_can.h | 4 +++
drivers/net/can/c_can/c_can_platform.c | 52 +++++++++++++++++++---------------
2 files changed, 33 insertions(+), 23 deletions(-)
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 99ad1aa..26c975d 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -169,6 +169,10 @@ enum c_can_dev_id {
BOSCH_D_CAN,
};
+struct c_can_driver_data {
+ enum c_can_dev_id id;
+};
+
/* c_can private data structure */
struct c_can_priv {
struct can_priv can; /* must be the first member */
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index b144e71..1546c2b 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -167,26 +167,34 @@ static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
}
}
+static struct c_can_driver_data c_can_drvdata = {
+ .id = BOSCH_C_CAN,
+};
+
+static struct c_can_driver_data d_can_drvdata = {
+ .id = BOSCH_D_CAN,
+};
+
static struct platform_device_id c_can_id_table[] = {
- [BOSCH_C_CAN_PLATFORM] = {
+ {
.name = KBUILD_MODNAME,
- .driver_data = BOSCH_C_CAN,
+ .driver_data = (kernel_ulong_t)&c_can_drvdata,
},
- [BOSCH_C_CAN] = {
+ {
.name = "c_can",
- .driver_data = BOSCH_C_CAN,
+ .driver_data = (kernel_ulong_t)&c_can_drvdata,
},
- [BOSCH_D_CAN] = {
+ {
.name = "d_can",
- .driver_data = BOSCH_D_CAN,
- }, {
- }
+ .driver_data = (kernel_ulong_t)&d_can_drvdata,
+ },
+ { /* sentinel */ },
};
MODULE_DEVICE_TABLE(platform, c_can_id_table);
static const struct of_device_id c_can_of_table[] = {
- { .compatible = "bosch,c_can", .data = &c_can_id_table[BOSCH_C_CAN] },
- { .compatible = "bosch,d_can", .data = &c_can_id_table[BOSCH_D_CAN] },
+ { .compatible = "bosch,c_can", .data = &c_can_drvdata },
+ { .compatible = "bosch,d_can", .data = &d_can_drvdata },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, c_can_of_table);
@@ -198,21 +206,19 @@ static int c_can_plat_probe(struct platform_device *pdev)
struct net_device *dev;
struct c_can_priv *priv;
const struct of_device_id *match;
- const struct platform_device_id *id;
struct resource *mem, *res;
int irq;
struct clk *clk;
-
- if (pdev->dev.of_node) {
- match = of_match_device(c_can_of_table, &pdev->dev);
- if (!match) {
- dev_err(&pdev->dev, "Failed to find matching dt id\n");
- ret = -EINVAL;
- goto exit;
- }
- id = match->data;
+ const struct c_can_driver_data *drvdata;
+
+ match = of_match_device(c_can_of_table, &pdev->dev);
+ if (match) {
+ drvdata = match->data;
+ } else if (pdev->id_entry->driver_data) {
+ drvdata = (struct c_can_driver_data *)
+ pdev->id_entry->driver_data;
} else {
- id = platform_get_device_id(pdev);
+ return -ENODEV;
}
/* get the appropriate clk */
@@ -244,7 +250,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
}
priv = netdev_priv(dev);
- switch (id->driver_data) {
+ switch (drvdata->id) {
case BOSCH_C_CAN:
priv->regs = reg_map_c_can;
switch (mem->flags & IORESOURCE_MEM_TYPE_MASK) {
@@ -303,7 +309,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
priv->device = &pdev->dev;
priv->can.clock.freq = clk_get_rate(clk);
priv->priv = clk;
- priv->type = id->driver_data;
+ priv->type = drvdata->id;
platform_set_drvdata(pdev, dev);
SET_NETDEV_DEV(dev, &pdev->dev);
--
1.8.3.2
^ permalink raw reply related
* [PATCH v4 5/8] net: can: c_can: Add support for START pulse in RAMINIT sequence
From: Roger Quadros @ 2014-11-07 14:49 UTC (permalink / raw)
To: wg, mkl
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros
In-Reply-To: <1415371762-29885-1-git-send-email-rogerq@ti.com>
Some SoCs e.g. (TI DRA7xx) need a START pulse to start the
RAMINIT sequence i.e. START bit must be set and cleared before
checking for the DONE bit status.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/net/can/c_can/c_can.h | 1 +
drivers/net/can/c_can/c_can_platform.c | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 0e17c7b..c6715ca 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -185,6 +185,7 @@ struct c_can_raminit {
unsigned int reg; /* register index within syscon */
u8 start_bit;
u8 done_bit;
+ bool needs_pulse;
};
/* c_can private data structure */
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 3776483..b838c6b 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -124,6 +124,12 @@ static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable)
ctrl |= 1 << start_bit;
regmap_write(raminit->syscon, raminit->reg, ctrl);
+ /* clear START bit if start pulse is needed */
+ if (raminit->needs_pulse) {
+ ctrl &= ~(1 << start_bit);
+ regmap_write(raminit->syscon, raminit->reg, ctrl);
+ }
+
ctrl |= 1 << done_bit;
c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
}
@@ -330,6 +336,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
raminit->start_bit = drvdata->raminit_start_bits[id];
raminit->done_bit = drvdata->raminit_done_bits[id];
+ raminit->needs_pulse = drvdata->raminit_pulse;
priv->raminit = c_can_hw_raminit_syscon;
} else {
--
1.8.3.2
^ permalink raw reply related
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