* Re: [PATCH v5] can: add Renesas R-Car CAN driver
From: Geert Uytterhoeven @ 2014-01-20 11:52 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Sergei Shtylyov, netdev@vger.kernel.org, wg, linux-can,
Linux-sh list, vksavl
In-Reply-To: <52DD0CC1.70205@pengutronix.de>
On Mon, Jan 20, 2014 at 12:47 PM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 01/20/2014 12:43 PM, Geert Uytterhoeven wrote:
>> On Thu, Dec 26, 2013 at 10:37 PM, Sergei Shtylyov
>> <sergei.shtylyov@cogentembedded.com> wrote:
>>> Changes in version 3:
>>
>>> - added '__packed' to 'struct rcar_can_mbox_regs' and 'struct rcar_can_regs';
>>> - removed unneeded type cast in the probe() method.
>>
>>> +/* Mailbox registers structure */
>>> +struct rcar_can_mbox_regs {
>>> + u32 id; /* IDE and RTR bits, SID and EID */
>>> + u8 stub; /* Not used */
>>> + u8 dlc; /* Data Length Code - bits [0..3] */
>>> + u8 data[8]; /* Data Bytes */
>>> + u8 tsh; /* Time Stamp Higher Byte */
>>> + u8 tsl; /* Time Stamp Lower Byte */
>>> +} __packed;
>>
>> Sorry, I missed the earlier discussion, but why the _packed?
>> One u32 and 12 bytes makes a nice multiple of 4.
>
> Better safe than sorry, it's the layout of the registers in hardware,
> don't let the compiler optimize here.
Actually __packed makes it less safe, as the compiler now assumes
the u32 id member is unaligned, and thus may turn 32-bit accesses into 4
byte accesses.
Fortunately it won't happen in this case as the code uses writel/readl to
acces the id member.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v5] can: add Renesas R-Car CAN driver
From: Marc Kleine-Budde @ 2014-01-20 11:47 UTC (permalink / raw)
To: Geert Uytterhoeven, Sergei Shtylyov
Cc: netdev@vger.kernel.org, wg, linux-can, Linux-sh list, vksavl
In-Reply-To: <CAMuHMdWZJUvcW9OzM39aB2m+VdKqmZ6EOGR_rZcAdV7Cmwx3kg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]
On 01/20/2014 12:43 PM, Geert Uytterhoeven wrote:
> Hi Sergei,
>
> On Thu, Dec 26, 2013 at 10:37 PM, Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
>> Changes in version 3:
>
>> - added '__packed' to 'struct rcar_can_mbox_regs' and 'struct rcar_can_regs';
>> - removed unneeded type cast in the probe() method.
>
>> +/* Mailbox registers structure */
>> +struct rcar_can_mbox_regs {
>> + u32 id; /* IDE and RTR bits, SID and EID */
>> + u8 stub; /* Not used */
>> + u8 dlc; /* Data Length Code - bits [0..3] */
>> + u8 data[8]; /* Data Bytes */
>> + u8 tsh; /* Time Stamp Higher Byte */
>> + u8 tsl; /* Time Stamp Lower Byte */
>> +} __packed;
>
> Sorry, I missed the earlier discussion, but why the _packed?
> One u32 and 12 bytes makes a nice multiple of 4.
Better safe than sorry, it's the layout of the registers in hardware,
don't let the compiler optimize here.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]
^ permalink raw reply
* [PATCH net-next v6 2/2] stmmac: Fix kernel crashes for jumbo frames
From: Vince Bridgers @ 2014-01-20 11:39 UTC (permalink / raw)
To: devicetree, netdev
Cc: peppe.cavallaro, robh+dt, pawel.moll, mark.rutland,
ijc+devicetree, galak, dinguyen, rayagond, vbridgers2013
In-Reply-To: <1390217941-22967-1-git-send-email-vbridgers2013@gmail.com>
These changes correct the following issues with jumbo frames on the
stmmac driver:
1) The Synopsys EMAC can be configured to support different FIFO
sizes at core configuration time. There's no way to query the
controller and know the FIFO size, so the driver needs to get this
information from the device tree in order to know how to correctly
handle MTU changes and setting up dma buffers. The default
max-frame-size is as currently used, which is the size of a jumbo
frame.
2) The driver was enabling Jumbo frames by default, but was not allocating
dma buffers of sufficient size to handle the maximum possible packet
size that could be received. This led to memory corruption since DMAs were
occurring beyond the extent of the allocated receive buffers for certain types
of network traffic.
kernel BUG at net/core/skbuff.c:126!
Internal error: Oops - BUG: 0 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 563 Comm: sockperf Not tainted 3.13.0-rc6-01523-gf7111b9 #31
task: ef35e580 ti: ef252000 task.ti: ef252000
PC is at skb_panic+0x60/0x64
LR is at skb_panic+0x60/0x64
pc : [<c03c7c3c>] lr : [<c03c7c3c>] psr: 60000113
sp : ef253c18 ip : 60000113 fp : 00000000
r10: ef3a5400 r9 : 00000ebc r8 : ef3a546c
r7 : ee59f000 r6 : ee59f084 r5 : ee59ff40 r4 : ee59f140
r3 : 000003e2 r2 : 00000007 r1 : c0b9c420 r0 : 0000007d
Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 10c5387d Table: 2e8ac04a DAC: 00000015
Process sockperf (pid: 563, stack limit = 0xef252248)
Stack: (0xef253c18 to 0xef254000)
3c00: 00000ebc ee59f000
3c20: ee59f084 ee59ff40 ee59f140 c04a9cd8 ee8c50c0 00000ebc ee59ff40 00000000
3c40: ee59f140 c02d0ef0 00000056 ef1eda80 ee8c50c0 00000ebc 22bbef29 c0318f8c
3c60: 00000056 ef3a547c ffe2c716 c02c9c90 c0ba1298 ef3a5838 ef3a5838 ef3a5400
3c80: 000020c0 ee573840 000055cb ef3f2050 c053f0e0 c0319214 22b9b085 22d92813
3ca0: 00001c80 004b8e00 ef3a5400 ee573840 ef3f2064 22d92813 ef3f2064 000055cb
3cc0: ef3f2050 c031a19c ef252000 00000000 00000000 c0561bc0 00000000 ff00ffff
3ce0: c05621c0 ef3a5400 ef3f2064 ee573840 00000020 ef3f2064 000055cb ef3f2050
3d00: c053f0e0 c031cad0 c053e740 00000e60 00000000 00000000 ee573840 ef3a5400
3d20: ef0a6e00 00000000 ef3f2064 c032507c 00010000 00000020 c0561bc0 c0561bc0
3d40: ee599850 c032799c 00000000 ee573840 c055a380 ef3a5400 00000000 ef3f2064
3d60: ef3f2050 c032799c 0101c7c0 2b6755cb c059a280 c030e4d8 000055cb ffffffff
3d80: ee574fc0 c055a380 ee574000 ee573840 00002b67 ee573840 c03fe9c4 c053fa68
3da0: c055a380 00001f6f 00000000 ee573840 c053f0e0 c0304fdc ef0a6e01 ef3f2050
3dc0: ee573858 ef031000 ee573840 c03055d8 c0ba0c40 ef000f40 00100100 c053f0dc
3de0: c053ffdc c053f0f0 00000008 00000000 ef031000 c02da948 00001140 00000000
3e00: c0563c78 ef253e5f 00000020 ee573840 00000020 c053f0f0 ef313400 ee573840
3e20: c053f0e0 00000000 00000000 c05380c0 ef313400 00001000 00000015 c02df280
3e40: ee574000 ef001e00 00000000 00001080 00000042 005cd980 ef031500 ef031500
3e60: 00000000 c02df824 ef031500 c053e390 c0541084 f00b1e00 c05925e8 c02df864
3e80: 00001f5c ef031440 c053e390 c0278524 00000002 00000000 c0b9eb48 c02df280
3ea0: ee8c7180 00000100 c0542ca8 00000015 00000040 ef031500 ef031500 ef031500
3ec0: c027803c ef252000 00000040 000000ec c05380c0 c0b9eb40 c0b9eb48 c02df940
3ee0: ef060780 ffffa4dd c0564a9c c056343c 002e80a8 00000080 ef031500 00000001
3f00: c053808c ef252000 fffec100 00000003 00000004 002e80a8 0000000c c00258f0
3f20: 002e80a8 c005e704 00000005 00000100 c05634d0 c0538080 c05333e0 00000000
3f40: 0000000a c0565580 c05380c0 ffffa4dc c05434f4 00400100 00000004 c0534cd4
3f60: 00000098 00000000 fffec100 002e80a8 00000004 002e80a8 002a20e0 c0025da8
3f80: c0534cd4 c000f020 fffec10c c053ea60 ef253fb0 c0008530 0000ffe2 b6ef67f4
3fa0: 40000010 ffffffff 00000124 c0012f3c 0000ffe2 002e80f0 0000ffe2 00004000
3fc0: becb6338 becb6334 00000004 00000124 002e80a8 00000004 002e80a8 002a20e0
3fe0: becb6300 becb62f4 002773bb b6ef67f4 40000010 ffffffff 00000000 00000000
[<c03c7c3c>] (skb_panic+0x60/0x64) from [<c02d0ef0>] (skb_put+0x4c/0x50)
[<c02d0ef0>] (skb_put+0x4c/0x50) from [<c0318f8c>] (tcp_collapse+0x314/0x3ec)
[<c0318f8c>] (tcp_collapse+0x314/0x3ec) from [<c0319214>]
(tcp_try_rmem_schedule+0x1b0/0x3c4)
[<c0319214>] (tcp_try_rmem_schedule+0x1b0/0x3c4) from [<c031a19c>]
(tcp_data_queue+0x480/0xe6c)
[<c031a19c>] (tcp_data_queue+0x480/0xe6c) from [<c031cad0>]
(tcp_rcv_established+0x180/0x62c)
[<c031cad0>] (tcp_rcv_established+0x180/0x62c) from [<c032507c>]
(tcp_v4_do_rcv+0x13c/0x31c)
[<c032507c>] (tcp_v4_do_rcv+0x13c/0x31c) from [<c032799c>]
(tcp_v4_rcv+0x718/0x73c)
[<c032799c>] (tcp_v4_rcv+0x718/0x73c) from [<c0304fdc>]
(ip_local_deliver+0x98/0x274)
[<c0304fdc>] (ip_local_deliver+0x98/0x274) from [<c03055d8>]
(ip_rcv+0x420/0x758)
[<c03055d8>] (ip_rcv+0x420/0x758) from [<c02da948>]
(__netif_receive_skb_core+0x44c/0x5bc)
[<c02da948>] (__netif_receive_skb_core+0x44c/0x5bc) from [<c02df280>]
(netif_receive_skb+0x48/0xb4)
[<c02df280>] (netif_receive_skb+0x48/0xb4) from [<c02df824>]
(napi_gro_flush+0x70/0x94)
[<c02df824>] (napi_gro_flush+0x70/0x94) from [<c02df864>]
(napi_complete+0x1c/0x34)
[<c02df864>] (napi_complete+0x1c/0x34) from [<c0278524>]
(stmmac_poll+0x4e8/0x5c8)
[<c0278524>] (stmmac_poll+0x4e8/0x5c8) from [<c02df940>]
(net_rx_action+0xc4/0x1e4)
[<c02df940>] (net_rx_action+0xc4/0x1e4) from [<c00258f0>]
(__do_softirq+0x12c/0x2e8)
[<c00258f0>] (__do_softirq+0x12c/0x2e8) from [<c0025da8>] (irq_exit+0x78/0xac)
[<c0025da8>] (irq_exit+0x78/0xac) from [<c000f020>] (handle_IRQ+0x44/0x90)
[<c000f020>] (handle_IRQ+0x44/0x90) from [<c0008530>]
(gic_handle_irq+0x2c/0x5c)
[<c0008530>] (gic_handle_irq+0x2c/0x5c) from [<c0012f3c>]
(__irq_usr+0x3c/0x60)
3) The driver was setting the dma buffer size after allocating dma buffers,
which caused a system panic when changing the MTU.
BUG: Bad page state in process ifconfig pfn:2e850
page:c0b72a00 count:0 mapcount:0 mapping: (null) index:0x0
page flags: 0x200(arch_1)
Modules linked in:
CPU: 0 PID: 566 Comm: ifconfig Not tainted 3.13.0-rc6-01523-gf7111b9 #29
[<c001547c>] (unwind_backtrace+0x0/0xf8) from [<c00122dc>]
(show_stack+0x10/0x14)
[<c00122dc>] (show_stack+0x10/0x14) from [<c03c793c>] (dump_stack+0x70/0x88)
[<c03c793c>] (dump_stack+0x70/0x88) from [<c00b2620>] (bad_page+0xc8/0x118)
[<c00b2620>] (bad_page+0xc8/0x118) from [<c00b302c>]
(get_page_from_freelist+0x744/0x870)
[<c00b302c>] (get_page_from_freelist+0x744/0x870) from [<c00b40f4>]
(__alloc_pages_nodemask+0x118/0x86c)
[<c00b40f4>] (__alloc_pages_nodemask+0x118/0x86c) from [<c00b4858>]
(__get_free_pages+0x10/0x54)
[<c00b4858>] (__get_free_pages+0x10/0x54) from [<c00cba1c>]
(kmalloc_order_trace+0x24/0xa0)
[<c00cba1c>] (kmalloc_order_trace+0x24/0xa0) from [<c02d199c>]
(__kmalloc_reserve.isra.21+0x24/0x70)
[<c02d199c>] (__kmalloc_reserve.isra.21+0x24/0x70) from [<c02d240c>]
(__alloc_skb+0x68/0x13c)
[<c02d240c>] (__alloc_skb+0x68/0x13c) from [<c02d3930>]
(__netdev_alloc_skb+0x3c/0xe8)
[<c02d3930>] (__netdev_alloc_skb+0x3c/0xe8) from [<c0279378>]
(stmmac_open+0x63c/0x1024)
[<c0279378>] (stmmac_open+0x63c/0x1024) from [<c02e18cc>]
(__dev_open+0xa0/0xfc)
[<c02e18cc>] (__dev_open+0xa0/0xfc) from [<c02e1b40>]
(__dev_change_flags+0x94/0x158)
[<c02e1b40>] (__dev_change_flags+0x94/0x158) from [<c02e1c24>]
(dev_change_flags+0x18/0x48)
[<c02e1c24>] (dev_change_flags+0x18/0x48) from [<c0337bc0>]
(devinet_ioctl+0x638/0x700)
[<c0337bc0>] (devinet_ioctl+0x638/0x700) from [<c02c7aec>]
(sock_ioctl+0x64/0x290)
[<c02c7aec>] (sock_ioctl+0x64/0x290) from [<c0100890>]
(do_vfs_ioctl+0x78/0x5b8)
[<c0100890>] (do_vfs_ioctl+0x78/0x5b8) from [<c0100e0c>] (SyS_ioctl+0x3c/0x5c)
[<c0100e0c>] (SyS_ioctl+0x3c/0x5c) from [<c000e760>]
The fixes have been verified using reproducible, automated testing.
Signed-off-by: Vince Bridgers <vbridgers2013@gmail.com>
---
V6: respin for updated net-next
V5: no change for this patch, change was in devicetree patch
V4: address inconsistency in comment, add comments explaining inconsistent
use of max-frame-size when reading device tree max-frame-size
V3: change snps,max-frame-size to max-frame-size
V2: change snps,max-mtu to snps,max-frame-size
---
drivers/net/ethernet/stmicro/stmmac/common.h | 4 +++-
drivers/net/ethernet/stmicro/stmmac/dwmac1000.h | 7 ++-----
.../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 7 ++++++-
.../net/ethernet/stmicro/stmmac/dwmac100_core.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 11 +++++++----
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 13 +++++++++++++
include/linux/stmmac.h | 1 +
7 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 5a60b3f..7834a39 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -292,6 +292,8 @@ struct dma_features {
#define STMMAC_CHAIN_MODE 0x1
#define STMMAC_RING_MODE 0x2
+#define JUMBO_LEN 9000
+
struct stmmac_desc_ops {
/* DMA RX descriptor ring initialization */
void (*init_rx_desc) (struct dma_desc *p, int disable_rx_ic, int mode,
@@ -368,7 +370,7 @@ struct stmmac_dma_ops {
struct stmmac_ops {
/* MAC core initialization */
- void (*core_init) (void __iomem *ioaddr);
+ void (*core_init) (void __iomem *ioaddr, int mtu);
/* Enable and verify that the IPC module is supported */
int (*rx_ipc) (void __iomem *ioaddr);
/* Dump MAC registers */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
index c12aabb..f37d90f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
@@ -126,11 +126,8 @@ enum power_event {
#define GMAC_ANE_PSE (3 << 7)
#define GMAC_ANE_PSE_SHIFT 7
- /* GMAC Configuration defines */
-#define GMAC_CONTROL_TC 0x01000000 /* Transmit Conf. in RGMII/SGMII */
-#define GMAC_CONTROL_WD 0x00800000 /* Disable Watchdog on receive */
-
/* GMAC Configuration defines */
+#define GMAC_CONTROL_2K 0x08000000 /* IEEE 802.3as 2K packets */
#define GMAC_CONTROL_TC 0x01000000 /* Transmit Conf. in RGMII/SGMII */
#define GMAC_CONTROL_WD 0x00800000 /* Disable Watchdog on receive */
#define GMAC_CONTROL_JD 0x00400000 /* Jabber disable */
@@ -156,7 +153,7 @@ enum inter_frame_gap {
#define GMAC_CONTROL_RE 0x00000004 /* Receiver Enable */
#define GMAC_CORE_INIT (GMAC_CONTROL_JD | GMAC_CONTROL_PS | GMAC_CONTROL_ACS | \
- GMAC_CONTROL_JE | GMAC_CONTROL_BE)
+ GMAC_CONTROL_BE)
/* GMAC Frame Filter defines */
#define GMAC_FRAME_FILTER_PR 0x00000001 /* Promiscuous Mode */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index cdd9268..b3e148e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -32,10 +32,15 @@
#include <asm/io.h>
#include "dwmac1000.h"
-static void dwmac1000_core_init(void __iomem *ioaddr)
+static void dwmac1000_core_init(void __iomem *ioaddr, int mtu)
{
u32 value = readl(ioaddr + GMAC_CONTROL);
value |= GMAC_CORE_INIT;
+ if (mtu > 1500)
+ value |= GMAC_CONTROL_2K;
+ if (mtu > 2000)
+ value |= GMAC_CONTROL_JE;
+
writel(value, ioaddr + GMAC_CONTROL);
/* Mask GMAC interrupts */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
index 5857d67..2ff767b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
@@ -32,7 +32,7 @@
#include <asm/io.h>
#include "dwmac100.h"
-static void dwmac100_core_init(void __iomem *ioaddr)
+static void dwmac100_core_init(void __iomem *ioaddr, int mtu)
{
u32 value = readl(ioaddr + MAC_CONTROL);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 5db91be..d93aa87 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -54,7 +54,6 @@
#include <linux/reset.h>
#define STMMAC_ALIGN(x) L1_CACHE_ALIGN(x)
-#define JUMBO_LEN 9000
/* Module parameters */
#define TX_TIMEO 5000
@@ -93,7 +92,7 @@ static int tc = TC_DEFAULT;
module_param(tc, int, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(tc, "DMA threshold control value");
-#define DMA_BUFFER_SIZE BUF_SIZE_2KiB
+#define DMA_BUFFER_SIZE BUF_SIZE_4KiB
static int buf_sz = DMA_BUFFER_SIZE;
module_param(buf_sz, int, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(buf_sz, "DMA buffer size");
@@ -994,6 +993,8 @@ static int init_dma_desc_rings(struct net_device *dev)
if (bfsize < BUF_SIZE_16KiB)
bfsize = stmmac_set_bfsize(dev->mtu, priv->dma_buf_sz);
+ priv->dma_buf_sz = bfsize;
+
if (netif_msg_probe(priv))
pr_debug("%s: txsize %d, rxsize %d, bfsize %d\n", __func__,
txsize, rxsize, bfsize);
@@ -1023,7 +1024,6 @@ static int init_dma_desc_rings(struct net_device *dev)
}
priv->cur_rx = 0;
priv->dirty_rx = (unsigned int)(i - rxsize);
- priv->dma_buf_sz = bfsize;
buf_sz = bfsize;
/* Setup the chained descriptor addresses */
@@ -1624,7 +1624,7 @@ static int stmmac_hw_setup(struct net_device *dev)
priv->plat->bus_setup(priv->ioaddr);
/* Initialize the MAC Core */
- priv->hw->mac->core_init(priv->ioaddr);
+ priv->hw->mac->core_init(priv->ioaddr, dev->mtu);
/* Enable the MAC Rx/Tx */
stmmac_set_mac(priv->ioaddr, true);
@@ -2274,6 +2274,9 @@ static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
else
max_mtu = SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN);
+ if (priv->plat->maxmtu < max_mtu)
+ max_mtu = priv->plat->maxmtu;
+
if ((new_mtu < 46) || (new_mtu > max_mtu)) {
pr_err("%s: invalid MTU, max MTU is: %d\n", dev->name, max_mtu);
return -EINVAL;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 9d4baa8..5884a7d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -102,6 +102,11 @@ static int stmmac_probe_config_dt(struct platform_device *pdev,
plat->force_sf_dma_mode = of_property_read_bool(np, "snps,force_sf_dma_mode");
+ /* Set the maxmtu to a default of JUMBO_LEN in case the
+ * parameter is not present in the device tree.
+ */
+ plat->maxmtu = JUMBO_LEN;
+
/*
* Currently only the properties needed on SPEAr600
* are provided. All other properties should be added
@@ -110,6 +115,14 @@ static int stmmac_probe_config_dt(struct platform_device *pdev,
if (of_device_is_compatible(np, "st,spear600-gmac") ||
of_device_is_compatible(np, "snps,dwmac-3.70a") ||
of_device_is_compatible(np, "snps,dwmac")) {
+ /* Note that the max-frame-size parameter as defined in the
+ * ePAPR v1.1 spec is defined as max-frame-size, it's
+ * actually used as the IEEE definition of MAC Client
+ * data, or MTU. The ePAPR specification is confusing as
+ * the definition is max-frame-size, but usage examples
+ * are clearly MTUs
+ */
+ of_property_read_u32(np, "max-frame-size", &plat->maxmtu);
plat->has_gmac = 1;
plat->pmt = 1;
}
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 1367974..6f27d4f 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -111,6 +111,7 @@ struct plat_stmmacenet_data {
int force_thresh_dma_mode;
int riwt_off;
int max_speed;
+ int maxmtu;
void (*fix_mac_speed)(void *priv, unsigned int speed);
void (*bus_setup)(void __iomem *ioaddr);
void *(*setup)(struct platform_device *pdev);
--
1.7.9.5
^ permalink raw reply related
* [PATCH net-next v6 1/2] dts: Add a binding for Synopsys emac max-frame-size
From: Vince Bridgers @ 2014-01-20 11:39 UTC (permalink / raw)
To: devicetree, netdev
Cc: peppe.cavallaro, robh+dt, pawel.moll, mark.rutland,
ijc+devicetree, galak, dinguyen, rayagond, vbridgers2013
In-Reply-To: <1390217941-22967-1-git-send-email-vbridgers2013@gmail.com>
This change adds a parameter for the Synopsys 10/100/1000
stmmac Ethernet driver to configure the maximum frame
size supported by the EMAC driver. Synopsys allows the FIFO
sizes to be configured when the cores are built for a particular
device, but do not provide a way for the driver to read
information from the device about the maximum MTU size
supported as limited by the device's FIFO size.
Signed-off-by: Vince Bridgers <vbridgers2013@gmail.com>
---
V6: respin for updated net-next
V5: simplify comment on use of max-frame-size
V4: add comments to explain use of max-frame-size with respect
to inconsistent definition and use in the ePAPR v1.1 spec
V3: change snps,max-frame-size to max-frame-size
V2: change snps,max-mtu to snps,max-frame-size
---
Documentation/devicetree/bindings/net/stmmac.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
index aefb639..9d92d42 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -32,6 +32,8 @@ Optional properties:
- resets: Should contain a phandle to the STMMAC reset signal, if any
- reset-names: Should contain the reset signal name "stmmaceth", if a
reset phandle is given
+- max-frame-size: Maximum Transfer Unit (IEEE defined MTU), rather
+ than the maximum frame size.
Examples:
@@ -42,5 +44,6 @@ Examples:
interrupts = <24 23>;
interrupt-names = "macirq", "eth_wake_irq";
mac-address = [000000000000]; /* Filled in by U-Boot */
+ max-frame-size = <3800>;
phy-mode = "gmii";
};
--
1.7.9.5
^ permalink raw reply related
* [PATCH net-next v6 0/2] stmmac: fix kernel crashes for jumbo frames
From: Vince Bridgers @ 2014-01-20 11:38 UTC (permalink / raw)
To: devicetree, netdev
Cc: peppe.cavallaro, robh+dt, pawel.moll, mark.rutland,
ijc+devicetree, galak, dinguyen, rayagond, vbridgers2013
v6:
* no changes since v5, respin for updated net-next
v5:
* Simplify use notes for max-frame-size in stmmac.txt, in the
devicetree patch. No change to the stmmac driver patch
from v4.
v4:
* Address inconsistent comment with use, add comments about inconsistency
of max-frame-size definition and use in the ePAPR v1.1 specification.
v3:
* change snps,max-frame-size to max-frame-size
v2:
* change snps,max-mtu to snps,max-frame-size
These patches address two kernel crashes seen when using jumbo frames on
the Synopsys stmmac driver, and adds device tree configurability for the
maximum mtu. The Synopsys emac fifo sizes can be configured when a logic
design is synthesized, but does not provide a way for a driver to query the
exact fifo size.
The crashes seen were due to two issues.
1) The dma buffer size was being set after the dma buffers were allocated.
This caused a crash when changing the mtu since it was possible the buffers
would subsequently be freed using an incorrect dma buffer size. This could
also cause kernel panics due to memory corruption since a large mtu size could
have been configured, but the dma buffers were not sized accordingly.
2) Jumbo frames were being enabled by default, but the dma buffers were not
sized accordingly. This caused memory corruption in the context of certain
types of network traffic, leading to kernel panics.
I've tested these changes using automated, reproducible testware. I can
demonstrate the panics described before the fixes and show that the fixes
address the problems described.
Testing and improvements continue through the use of the mentioned automated
and reproducible testware.
Vince Bridgers
Vince Bridgers (2):
dts: Add a binding for Synopsys emac max-frame-size
stmmac: Fix kernel crashes for jumbo frames
Documentation/devicetree/bindings/net/stmmac.txt | 3 +++
drivers/net/ethernet/stmicro/stmmac/common.h | 4 +++-
drivers/net/ethernet/stmicro/stmmac/dwmac1000.h | 7 ++-----
.../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 7 ++++++-
.../net/ethernet/stmicro/stmmac/dwmac100_core.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 11 +++++++----
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 13 +++++++++++++
include/linux/stmmac.h | 1 +
8 files changed, 36 insertions(+), 12 deletions(-)
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH v5] can: add Renesas R-Car CAN driver
From: Geert Uytterhoeven @ 2014-01-20 11:43 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: netdev@vger.kernel.org, wg, mkl, linux-can, Linux-sh list, vksavl
In-Reply-To: <201312270037.15822.sergei.shtylyov@cogentembedded.com>
Hi Sergei,
On Thu, Dec 26, 2013 at 10:37 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Changes in version 3:
> - added '__packed' to 'struct rcar_can_mbox_regs' and 'struct rcar_can_regs';
> - removed unneeded type cast in the probe() method.
> +/* Mailbox registers structure */
> +struct rcar_can_mbox_regs {
> + u32 id; /* IDE and RTR bits, SID and EID */
> + u8 stub; /* Not used */
> + u8 dlc; /* Data Length Code - bits [0..3] */
> + u8 data[8]; /* Data Bytes */
> + u8 tsh; /* Time Stamp Higher Byte */
> + u8 tsl; /* Time Stamp Lower Byte */
> +} __packed;
Sorry, I missed the earlier discussion, but why the _packed?
One u32 and 12 bytes makes a nice multiple of 4.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH net-next V4 3/3] net: Add GRO support for vxlan traffic
From: Or Gerlitz @ 2014-01-20 11:40 UTC (permalink / raw)
To: Tom Herbert
Cc: Or Gerlitz, David Miller, Linux Netdev List, Jerry Chu,
Eric Dumazet, Herbert Xu, Yan Burman, Shlomo Pongratz
In-Reply-To: <CAJZOPZJP7NGdQnHs1tVXny19w4mstEaEBi5WEvktZ4MumCfy0w@mail.gmail.com>
On Tue, Jan 14, 2014 at 11:47 PM, Or Gerlitz <or.gerlitz@gmail.com> wrote:
> On Tue, Jan 14, 2014 at 7:59 PM, Tom Herbert <therbert@google.com> wrote:
>> On Tue, Jan 14, 2014 at 8:00 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
>> > Add GRO handlers for vxlann, by using the UDP GRO infrastructure.
>
> [...]
>>
>> > /* Notify netdevs that UDP port started listening */
>> > -static void vxlan_notify_add_rx_port(struct sock *sk)
>> > +static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
>> > {
>> > struct net_device *dev;
>> > + struct sock *sk = vs->sock->sk;
>> > struct net *net = sock_net(sk);
>> > sa_family_t sa_family = sk->sk_family;
>> > __be16 port = inet_sk(sk)->inet_sport;
>> > + int err;
>> > +
>> > + if (sa_family == AF_INET) {
>
>> Is this necessary? What about support for AF_INET6?
>
> Point taken -- the UDP GRO code for both ipv4 and ipv6 would work the same.
>
> So we can export udp_gro_receive/complete from net/ipv4/udp_offload.c
> such that they can be referred from net/ipv6/udp_offload.c
Hi Tom,
Spinning heads on this little more, it seems that the extension to IPv6 is
pretty much doable but needs little more work. I would prefer it to
be carried out
incrementally once the basic concept + IPv4 support, is merged.
So posting V5 now with the RCU directives set by Eric fixed.
Or.
^ permalink raw reply
* Re: [PATCH net-next 0/2] sctp: some small clean ups
From: Daniel Borkmann @ 2014-01-20 11:37 UTC (permalink / raw)
To: Wang Weidong; +Cc: nhorman, davem, vyasevich, netdev, linux-sctp
In-Reply-To: <1390217247-9408-1-git-send-email-wangweidong1@huawei.com>
On 01/20/2014 12:27 PM, Wang Weidong wrote:
> We have the macros in sctp.h, so use them for coding accordance
> in sctp.
Thanks for doing this Wang.
I am actually wondering why we have these macro locking wrappers
and not use these functions directly? Hm, any reasons? Maybe we
should rather go in the other direction with this?
> Wang Weidong (2):
> sctp: use sctp_local_bh_{disable|enable} instead
> local_bh_{disable|enable}
> sctp: use sctp_read_[un]lock instead of read_[un]lock
>
> net/sctp/endpointola.c | 4 ++--
> net/sctp/input.c | 10 +++++-----
> net/sctp/proc.c | 12 ++++++------
> net/sctp/sm_make_chunk.c | 8 ++++----
> net/sctp/socket.c | 8 ++++----
> 5 files changed, 21 insertions(+), 21 deletions(-)
>
^ permalink raw reply
* Re: [PATCH linux-next] net: batman-adv: use "__packed __aligned(2)" for each structure instead of "__packed(2)" region
From: James Hogan @ 2014-01-20 11:28 UTC (permalink / raw)
To: Antonio Quartulli
Cc: Chen Gang, David Miller, mareklindner, sw, b.a.t.m.a.n, netdev,
linux-kernel@vger.kernel.org, linux-metag
In-Reply-To: <52DB9B39.9090502@meshcoding.com>
[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]
Hi Antonio,
On 19/01/14 09:30, Antonio Quartulli wrote:
> On 19/01/14 02:10, James Hogan wrote:
>> It appears that the following gcc patch adds support for #pragma pack:
>> http://gcc.gnu.org/ml/gcc-patches/2006-10/msg01115.html
>>
>> I gave it a quick spin on metag gcc (which is unfortunately stuck on an old
>> version) and it seems to fix my simple test case so that #pragma pack(2)
>> becomes equivalent to __packed __aligned(2) (for sizeof and __alignof__).
>>
>
> Then I personally think that it is better to fix metag gcc instead of
> changing the kernel.
Indeed it makes sense to patch metag gcc to be safe in the presence of
unportable code like this.
> Actually there are many different spots where "#pragma pack" is used.
> batman-adv is just the only one having compile time checks for structure
> sizes.
Well the only vaguely interesting one I can find outside of drivers is
in fs/udf, and even that seems specific to CD-ROMs and DVDs.
If you care about portability then Chen's patch looks reasonable to me.
Cheers
James
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH net-next 2/2] sctp: use sctp_read_[un]lock instead of read_[un]lock
From: Wang Weidong @ 2014-01-20 11:27 UTC (permalink / raw)
To: nhorman, davem, vyasevich; +Cc: dborkman, netdev, linux-sctp
In-Reply-To: <1390217247-9408-1-git-send-email-wangweidong1@huawei.com>
While we have the macros sctp_read_[un]lock, so use them.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/endpointola.c | 4 ++--
net/sctp/input.c | 10 +++++-----
net/sctp/proc.c | 12 ++++++------
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index 6ffb6c1..68e1f91 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -342,7 +342,7 @@ static struct sctp_association *__sctp_endpoint_lookup_assoc(
hash = sctp_assoc_hashfn(sock_net(ep->base.sk), ep->base.bind_addr.port,
rport);
head = &sctp_assoc_hashtable[hash];
- read_lock(&head->lock);
+ sctp_read_lock(&head->lock);
sctp_for_each_hentry(epb, &head->chain) {
tmp = sctp_assoc(epb);
if (tmp->ep != ep || rport != tmp->peer.port)
@@ -355,7 +355,7 @@ static struct sctp_association *__sctp_endpoint_lookup_assoc(
break;
}
}
- read_unlock(&head->lock);
+ sctp_read_unlock(&head->lock);
out:
return asoc;
}
diff --git a/net/sctp/input.c b/net/sctp/input.c
index 1f4eeb4..f22669c 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -768,7 +768,7 @@ static struct sctp_endpoint *__sctp_rcv_lookup_endpoint(struct net *net,
hash = sctp_ep_hashfn(net, ntohs(laddr->v4.sin_port));
head = &sctp_ep_hashtable[hash];
- read_lock(&head->lock);
+ sctp_read_lock(&head->lock);
sctp_for_each_hentry(epb, &head->chain) {
ep = sctp_ep(epb);
if (sctp_endpoint_is_match(ep, net, laddr))
@@ -779,7 +779,7 @@ static struct sctp_endpoint *__sctp_rcv_lookup_endpoint(struct net *net,
hit:
sctp_endpoint_hold(ep);
- read_unlock(&head->lock);
+ sctp_read_unlock(&head->lock);
return ep;
}
@@ -863,7 +863,7 @@ static struct sctp_association *__sctp_lookup_association(
hash = sctp_assoc_hashfn(net, ntohs(local->v4.sin_port),
ntohs(peer->v4.sin_port));
head = &sctp_assoc_hashtable[hash];
- read_lock(&head->lock);
+ sctp_read_lock(&head->lock);
sctp_for_each_hentry(epb, &head->chain) {
asoc = sctp_assoc(epb);
transport = sctp_assoc_is_match(asoc, net, local, peer);
@@ -871,14 +871,14 @@ static struct sctp_association *__sctp_lookup_association(
goto hit;
}
- read_unlock(&head->lock);
+ sctp_read_unlock(&head->lock);
return NULL;
hit:
*pt = transport;
sctp_association_hold(asoc);
- read_unlock(&head->lock);
+ sctp_read_unlock(&head->lock);
return asoc;
}
diff --git a/net/sctp/proc.c b/net/sctp/proc.c
index 63ba0bd..bbba718 100644
--- a/net/sctp/proc.c
+++ b/net/sctp/proc.c
@@ -219,7 +219,7 @@ static int sctp_eps_seq_show(struct seq_file *seq, void *v)
head = &sctp_ep_hashtable[hash];
sctp_local_bh_disable();
- read_lock(&head->lock);
+ sctp_read_lock(&head->lock);
sctp_for_each_hentry(epb, &head->chain) {
ep = sctp_ep(epb);
sk = epb->sk;
@@ -234,7 +234,7 @@ static int sctp_eps_seq_show(struct seq_file *seq, void *v)
sctp_seq_dump_local_addrs(seq, epb);
seq_printf(seq, "\n");
}
- read_unlock(&head->lock);
+ sctp_read_unlock(&head->lock);
sctp_local_bh_enable();
return 0;
@@ -327,7 +327,7 @@ static int sctp_assocs_seq_show(struct seq_file *seq, void *v)
head = &sctp_assoc_hashtable[hash];
sctp_local_bh_disable();
- read_lock(&head->lock);
+ sctp_read_lock(&head->lock);
sctp_for_each_hentry(epb, &head->chain) {
assoc = sctp_assoc(epb);
sk = epb->sk;
@@ -361,7 +361,7 @@ static int sctp_assocs_seq_show(struct seq_file *seq, void *v)
sk->sk_rcvbuf);
seq_printf(seq, "\n");
}
- read_unlock(&head->lock);
+ sctp_read_unlock(&head->lock);
sctp_local_bh_enable();
return 0;
@@ -447,7 +447,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
head = &sctp_assoc_hashtable[hash];
sctp_local_bh_disable();
- read_lock(&head->lock);
+ sctp_read_lock(&head->lock);
rcu_read_lock();
sctp_for_each_hentry(epb, &head->chain) {
if (!net_eq(sock_net(epb->sk), seq_file_net(seq)))
@@ -504,7 +504,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v)
}
rcu_read_unlock();
- read_unlock(&head->lock);
+ sctp_read_unlock(&head->lock);
sctp_local_bh_enable();
return 0;
--
1.7.12
^ permalink raw reply related
* [PATCH net-next 0/2] sctp: some small clean ups
From: Wang Weidong @ 2014-01-20 11:27 UTC (permalink / raw)
To: nhorman, davem, vyasevich; +Cc: dborkman, netdev, linux-sctp
We have the macros in sctp.h, so use them for coding accordance
in sctp.
Wang Weidong (2):
sctp: use sctp_local_bh_{disable|enable} instead
local_bh_{disable|enable}
sctp: use sctp_read_[un]lock instead of read_[un]lock
net/sctp/endpointola.c | 4 ++--
net/sctp/input.c | 10 +++++-----
net/sctp/proc.c | 12 ++++++------
net/sctp/sm_make_chunk.c | 8 ++++----
net/sctp/socket.c | 8 ++++----
5 files changed, 21 insertions(+), 21 deletions(-)
--
1.7.12
^ permalink raw reply
* [PATCH net-next 1/2] sctp: use sctp_local_bh_{disable|enable} instead local_bh_{disable|enable}
From: Wang Weidong @ 2014-01-20 11:27 UTC (permalink / raw)
To: nhorman, davem, vyasevich; +Cc: dborkman, netdev, linux-sctp
In-Reply-To: <1390217247-9408-1-git-send-email-wangweidong1@huawei.com>
While we have the macros sctp_local_bh_{disable|enable}, so use them.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/sm_make_chunk.c | 8 ++++----
net/sctp/socket.c | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 632090b..1bbac08 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -3266,12 +3266,12 @@ static void sctp_asconf_param_success(struct sctp_association *asoc,
/* This is always done in BH context with a socket lock
* held, so the list can not change.
*/
- local_bh_disable();
+ sctp_local_bh_disable();
list_for_each_entry(saddr, &bp->address_list, list) {
if (sctp_cmp_addr_exact(&saddr->a, &addr))
saddr->state = SCTP_ADDR_SRC;
}
- local_bh_enable();
+ sctp_local_bh_enable();
list_for_each_entry(transport, &asoc->peer.transport_addr_list,
transports) {
dst_release(transport->dst);
@@ -3279,14 +3279,14 @@ static void sctp_asconf_param_success(struct sctp_association *asoc,
}
break;
case SCTP_PARAM_DEL_IP:
- local_bh_disable();
+ sctp_local_bh_disable();
sctp_del_bind_addr(bp, &addr);
if (asoc->asconf_addr_del_pending != NULL &&
sctp_cmp_addr_exact(asoc->asconf_addr_del_pending, &addr)) {
kfree(asoc->asconf_addr_del_pending);
asoc->asconf_addr_del_pending = NULL;
}
- local_bh_enable();
+ sctp_local_bh_enable();
list_for_each_entry(transport, &asoc->peer.transport_addr_list,
transports) {
dst_release(transport->dst);
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index fd7337a..f73918c 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3985,7 +3985,7 @@ static int sctp_init_sock(struct sock *sk)
SCTP_DBG_OBJCNT_INC(sock);
- local_bh_disable();
+ sctp_local_bh_disable();
percpu_counter_inc(&sctp_sockets_allocated);
sock_prot_inuse_add(net, sk->sk_prot, 1);
if (net->sctp.default_auto_asconf) {
@@ -3994,7 +3994,7 @@ static int sctp_init_sock(struct sock *sk)
sp->do_auto_asconf = 1;
} else
sp->do_auto_asconf = 0;
- local_bh_enable();
+ sctp_local_bh_enable();
return 0;
}
@@ -4019,10 +4019,10 @@ static void sctp_destroy_sock(struct sock *sk)
list_del(&sp->auto_asconf_list);
}
sctp_endpoint_free(sp->ep);
- local_bh_disable();
+ sctp_local_bh_disable();
percpu_counter_dec(&sctp_sockets_allocated);
sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
- local_bh_enable();
+ sctp_local_bh_enable();
}
/* Triggered when there are no references on the socket anymore */
--
1.7.12
^ permalink raw reply related
* RE: [PATCH 2/4 ethtool] ethtool: Support for configurable RSS hash key.
From: Venkata Duvvuru @ 2014-01-20 11:15 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev@vger.kernel.org
In-Reply-To: <1390156549.16433.119.camel@deadeye.wl.decadent.org.uk>
Ben, Thanks for the feedback.
-----Original Message-----
From: Ben Hutchings [mailto:ben@decadent.org.uk]
Sent: Monday, January 20, 2014 12:06 AM
To: Venkata Duvvuru
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH 2/4 ethtool] ethtool: Support for configurable RSS hash key.
On Fri, 2014-01-17 at 13:02 +0000, Venkata Duvvuru wrote:
> This ethtool patch will primarily implement the parser for the options provided by the user for set and get hashkey before invoking the ioctl.
> This patch also has Ethtool man page changes which describes the Usage of set and get hashkey options.
I'd prefer to have this combined with the -x/-X options (and add new long options to reflect that they cover the key as well).
if we add hashkey options to the existing -x/-X (--show-rxfh-indir/ --set-rxfh-indir), I think it won't be appropriate going by the command name.
We could change the command name to something like --show-rssconfig /--rss-config but I'm afraid would that be backward compatible?
[...]
> diff --git a/ethtool.c b/ethtool.c
> index b06dfa3..4b05b0c 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -471,6 +471,59 @@ static int rxflow_str_to_type(const char *str)
> return flow_type;
> }
>
> +static inline int is_hkey_char_valid(const char rss_hkey_string) {
A char is not a string.
> + /* Are there any invalid characters in the string */
> + return ((rss_hkey_string >= '0' && rss_hkey_string <= '9') ||
> + (rss_hkey_string >= 'a' && rss_hkey_string <= 'f') ||
> + (rss_hkey_string >= 'A' && rss_hkey_string <= 'F')); }
Braces are in the wrong places. And the whole function is redundant with isxdigit() anyway.
> +static int convert_string_to_hashkey(struct ethtool_rss_hkey *rss_hkey,
> + const char *rss_hkey_string) {
> + int i = 0;
> + int hex_byte;
> +
> + do {
> + if (i > (RSS_HASH_KEY_LEN - 1)) {
Comparing with the wrong limit.
[...]
> +static int get_hashkey(struct cmd_context *ctx) {
Brace in the wrong place.
[...]
> + for (i = 0; i < RSS_HASH_KEY_LEN; i++) {
> + if (i == (RSS_HASH_KEY_LEN - 1))
Wrong length.
> + printf("%02x\n", rss_hkey->data[i]);
> + else
> + printf("%02x:", rss_hkey->data[i]);
> + }
> +
> +done:
> + free(rss_hkey);
> + return rc;
> +}
[...]
--
Ben Hutchings
friends: People who know you well, but like you anyway.
^ permalink raw reply
* [PATCH v3 5/7] net: moxa: add IFF_LIVE_ADDR_CHANGE flag
From: Jonas Jensen @ 2014-01-20 11:13 UTC (permalink / raw)
To: netdev
Cc: linux-arm-kernel, linux-kernel, davem, f.fainelli, bhutchings,
Jonas Jensen
In-Reply-To: <1390216399-27028-1-git-send-email-jonas.jensen@gmail.com>
.ndo_set_mac_address hook callback already supports IFF_LIVE_ADDR_CHANGE
so add it to our flags.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Applies to next-20140120
drivers/net/ethernet/moxa/moxart_ether.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index ca892bb..6df372f 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -728,7 +728,7 @@ static int moxart_mac_probe(struct platform_device *pdev)
ether_setup(ndev);
ndev->netdev_ops = &moxart_netdev_ops;
netif_napi_add(ndev, &priv->napi, moxart_rx_poll, RX_DESC_NUM);
- ndev->priv_flags |= IFF_UNICAST_FLT;
+ ndev->priv_flags |= IFF_UNICAST_FLT | IFF_LIVE_ADDR_CHANGE;
ndev->irq = irq;
SET_NETDEV_DEV(ndev, &pdev->dev);
--
1.8.2.1
^ permalink raw reply related
* [PATCH v3 4/7] net: moxa: add ethtool support
From: Jonas Jensen @ 2014-01-20 11:13 UTC (permalink / raw)
To: netdev
Cc: linux-arm-kernel, linux-kernel, davem, f.fainelli, bhutchings,
Jonas Jensen
In-Reply-To: <1390216399-27028-1-git-send-email-jonas.jensen@gmail.com>
Add and assign ethtool_ops callback functions.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Applies to next-20140120
drivers/net/ethernet/moxa/moxart_ether.c | 121 +++++++++++++++++++++++++++++++
1 file changed, 121 insertions(+)
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index c19bff2..ca892bb 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -31,6 +31,10 @@
#include "moxart_ether.h"
+#define DRV_NAME "moxart-ethernet"
+#define DRV_VERSION "0.2"
+#define MOXART_NUM_STATS ARRAY_SIZE(ethtool_stats_keys)
+
static inline void moxart_emac_write(struct net_device *ndev,
unsigned int reg, unsigned long value)
{
@@ -63,6 +67,122 @@ static int moxart_set_mac_address(struct net_device *ndev, void *addr)
return 0;
}
+static struct {
+ const char str[ETH_GSTRING_LEN];
+} ethtool_stats_keys[] = {
+ { "tx_ok_mcol_2to15" },
+ { "tx_ok_1col" },
+ { "rx_frame_pause" },
+ { "frame_align_err" },
+ { "err_col_late_16" },
+ { "err_col_16" },
+ { "rx_runt" },
+ { "late_col" },
+ { "crc_err" },
+ { "rx_ftl" },
+ { "rx_fifo_full" },
+ { "rx_col" },
+ { "rx_bcast" },
+ { "rx_mcast" },
+ { "rx_ok" },
+ { "tx_ok" },
+};
+
+static void moxart_get_drvinfo(struct net_device *ndev,
+ struct ethtool_drvinfo *info)
+{
+ strlcpy(info->driver, DRV_NAME, sizeof(info->driver));
+ strlcpy(info->version, DRV_VERSION, sizeof(info->version));
+ strlcpy(info->bus_info, dev_name(&ndev->dev), sizeof(info->bus_info));
+}
+
+static int moxart_get_settings(struct net_device *ndev, struct ethtool_cmd *cmd)
+{
+ struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+ return phy_ethtool_gset(priv->phy_dev, cmd);
+}
+
+static int moxart_set_settings(struct net_device *ndev, struct ethtool_cmd *cmd)
+{
+ struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+ return phy_ethtool_sset(priv->phy_dev, cmd);
+}
+
+static int moxart_nway_reset(struct net_device *ndev)
+{
+ struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+ return genphy_restart_aneg(priv->phy_dev);
+}
+
+static void moxart_get_ethtool_stats(struct net_device *ndev,
+ struct ethtool_stats *estats,
+ u64 *tmp_stats)
+{
+ struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+ u32 s;
+ int i = 0;
+
+ s = readl(priv->base + REG_TX_COL_COUNTER);
+ tmp_stats[i++] = s & 0xffff0000;
+ tmp_stats[i++] = s & 0x0000ffff;
+ s = readl(priv->base + REG_RPF_AEP_COUNTER);
+ tmp_stats[i++] = s & 0xffff0000;
+ tmp_stats[i++] = s & 0x0000ffff;
+ s = readl(priv->base + REG_XM_PG_COUNTER);
+ tmp_stats[i++] = s & 0xffff0000;
+ tmp_stats[i++] = s & 0x0000ffff;
+ s = readl(priv->base + REG_RUNT_TLC_COUNTER);
+ tmp_stats[i++] = s & 0xffff0000;
+ tmp_stats[i++] = s & 0x0000ffff;
+ s = readl(priv->base + REG_CRC_FTL_COUNTER);
+ tmp_stats[i++] = s & 0xffff0000;
+ tmp_stats[i++] = s & 0x0000ffff;
+ s = readl(priv->base + REG_RLC_RCC_COUNTER);
+ tmp_stats[i++] = s & 0xffff0000;
+ tmp_stats[i++] = s & 0x0000ffff;
+ tmp_stats[i++] = readl(priv->base + REG_BROC_COUNTER);
+ tmp_stats[i++] = readl(priv->base + REG_MULCA_COUNTER);
+ tmp_stats[i++] = readl(priv->base + REG_XP_COUNTER);
+ tmp_stats[i++] = readl(priv->base + REG_RP_COUNTER);
+}
+
+static int moxart_get_sset_count(struct net_device *netdev,
+ int string_set)
+{
+ switch (string_set) {
+ case ETH_SS_STATS:
+ return MOXART_NUM_STATS;
+ default:
+ return -EINVAL;
+ }
+}
+
+static void moxart_get_strings(struct net_device *dev, u32 stringset, u8 *data)
+{
+ switch (stringset) {
+ case ETH_SS_STATS:
+ memcpy(data, ðtool_stats_keys, sizeof(ethtool_stats_keys));
+ break;
+ default:
+ WARN_ON(1);
+ break;
+ }
+}
+
+static const struct ethtool_ops moxart_ethtool_ops = {
+ .set_settings = moxart_set_settings,
+ .get_settings = moxart_get_settings,
+ .get_drvinfo = moxart_get_drvinfo,
+ .nway_reset = moxart_nway_reset,
+ .get_link = ethtool_op_get_link,
+ .get_ethtool_stats = moxart_get_ethtool_stats,
+ .get_sset_count = moxart_get_sset_count,
+ .get_strings = moxart_get_strings,
+};
+
static int moxart_do_ioctl(struct net_device *ndev, struct ifreq *ir, int cmd)
{
struct moxart_mac_priv_t *priv = netdev_priv(ndev);
@@ -612,6 +732,7 @@ static int moxart_mac_probe(struct platform_device *pdev)
ndev->irq = irq;
SET_NETDEV_DEV(ndev, &pdev->dev);
+ SET_ETHTOOL_OPS(ndev, &moxart_ethtool_ops);
ret = register_netdev(ndev);
if (ret) {
--
1.8.2.1
^ permalink raw reply related
* [PATCH v3 7/7] net: moxa: use eth_mac_addr()
From: Jonas Jensen @ 2014-01-20 11:13 UTC (permalink / raw)
To: netdev
Cc: linux-arm-kernel, linux-kernel, davem, f.fainelli, bhutchings,
Jonas Jensen
In-Reply-To: <1390216399-27028-1-git-send-email-jonas.jensen@gmail.com>
Replace boilerplate in moxart_set_mac_address() with eth_mac_addr().
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Applies to next-20140120
drivers/net/ethernet/moxa/moxart_ether.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index 0f25f4dc..8378be9 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -56,12 +56,7 @@ static void moxart_update_mac_address(struct net_device *ndev)
static int moxart_set_mac_address(struct net_device *ndev, void *addr)
{
- struct sockaddr *address = addr;
-
- if (!is_valid_ether_addr(address->sa_data))
- return -EADDRNOTAVAIL;
-
- memcpy(ndev->dev_addr, address->sa_data, ndev->addr_len);
+ eth_mac_addr(ndev, addr);
moxart_update_mac_address(ndev);
return 0;
--
1.8.2.1
^ permalink raw reply related
* [PATCH v3 6/7] net: moxa: generate random address
From: Jonas Jensen @ 2014-01-20 11:13 UTC (permalink / raw)
To: netdev
Cc: linux-arm-kernel, linux-kernel, davem, f.fainelli, bhutchings,
Jonas Jensen
In-Reply-To: <1390216399-27028-1-git-send-email-jonas.jensen@gmail.com>
The address register is zero:ed on boot, fill it with a randomly generated
address on probe.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Applies to next-20140120
drivers/net/ethernet/moxa/moxart_ether.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index 6df372f..0f25f4dc 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -740,6 +740,12 @@ static int moxart_mac_probe(struct platform_device *pdev)
goto init_fail;
}
+ if (!is_valid_ether_addr(ndev->dev_addr)) {
+ eth_hw_addr_random(ndev);
+ netdev_info(ndev, "generated random MAC address %pM\n",
+ ndev->dev_addr);
+ }
+
netdev_dbg(ndev, "%s: IRQ=%d address=%pM\n",
__func__, ndev->irq, ndev->dev_addr);
--
1.8.2.1
^ permalink raw reply related
* [PATCH v3 3/7] net: moxa: connect to PHY
From: Jonas Jensen @ 2014-01-20 11:13 UTC (permalink / raw)
To: netdev
Cc: linux-arm-kernel, linux-kernel, davem, f.fainelli, bhutchings,
Jonas Jensen
In-Reply-To: <1390216399-27028-1-git-send-email-jonas.jensen@gmail.com>
The kernel now has a MDIO bus driver and a phy_driver (RTL8201CP),
connect to this PHY using OF.
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Applies to next-20140120
.../devicetree/bindings/net/moxa,moxart-mac.txt | 47 ++++++++++-
drivers/net/ethernet/moxa/moxart_ether.c | 92 +++++++++++++++++++++-
drivers/net/ethernet/moxa/moxart_ether.h | 2 +
3 files changed, 138 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/moxa,moxart-mac.txt b/Documentation/devicetree/bindings/net/moxa,moxart-mac.txt
index 583418b..94c1f3b 100644
--- a/Documentation/devicetree/bindings/net/moxa,moxart-mac.txt
+++ b/Documentation/devicetree/bindings/net/moxa,moxart-mac.txt
@@ -1,21 +1,64 @@
MOXA ART Ethernet Controller
+Integrated MDIO bus node:
+
+- compatible: "moxa,moxart-mdio"
+- Inherits from MDIO bus node binding[1]
+
+[1] Documentation/devicetree/bindings/net/phy.txt
+
+
+Ethernet node:
+
Required properties:
- compatible : Must be "moxa,moxart-mac"
- reg : Should contain register location and length
- interrupts : Should contain the mac interrupt number
+Optional Properties:
+
+- phy-handle : the phandle to a PHY node
+
+
Example:
+ mdio0: mdio@90900090 {
+ compatible = "moxa,moxart-mdio";
+ reg = <0x90900090 0x8>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethphy0: ethernet-phy@1 {
+ device_type = "ethernet-phy";
+ compatible = "moxa,moxart-rtl8201cp", "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ };
+ };
+
+ mdio1: mdio@92000090 {
+ compatible = "moxa,moxart-mdio";
+ reg = <0x92000090 0x8>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethphy1: ethernet-phy@1 {
+ device_type = "ethernet-phy";
+ compatible = "moxa,moxart-rtl8201cp", "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ };
+ };
+
mac0: mac@90900000 {
compatible = "moxa,moxart-mac";
- reg = <0x90900000 0x100>;
+ reg = <0x90900000 0x90>;
interrupts = <25 0>;
+ phy-handle = <ðphy0>;
};
mac1: mac@92000000 {
compatible = "moxa,moxart-mac";
- reg = <0x92000000 0x100>;
+ reg = <0x92000000 0x90>;
interrupts = <27 0>;
+ phy-handle = <ðphy1>;
};
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index 17c9f0e..c19bff2 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -25,6 +25,9 @@
#include <linux/of_irq.h>
#include <linux/crc32.h>
#include <linux/crc32c.h>
+#include <linux/phy.h>
+#include <linux/of_mdio.h>
+#include <linux/of_net.h>
#include "moxart_ether.h"
@@ -60,6 +63,16 @@ static int moxart_set_mac_address(struct net_device *ndev, void *addr)
return 0;
}
+static int moxart_do_ioctl(struct net_device *ndev, struct ifreq *ir, int cmd)
+{
+ struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+ if (!netif_running(ndev))
+ return -EINVAL;
+
+ return phy_mii_ioctl(priv->phy_dev, ir, cmd);
+}
+
static void moxart_mac_free_memory(struct net_device *ndev)
{
struct moxart_mac_priv_t *priv = netdev_priv(ndev);
@@ -109,6 +122,19 @@ static void moxart_mac_enable(struct net_device *ndev)
writel(priv->reg_maccr, priv->base + REG_MAC_CTRL);
}
+static void moxart_mac_update_duplex(struct net_device *ndev)
+{
+ struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+ priv->reg_maccr &= ~(FULLDUP | ENRX_IN_HALFTX);
+ if (priv->duplex)
+ priv->reg_maccr |= FULLDUP;
+ else
+ priv->reg_maccr |= ENRX_IN_HALFTX;
+
+ writel(priv->reg_maccr, priv->base + REG_MAC_CTRL);
+}
+
static void moxart_mac_setup_desc_ring(struct net_device *ndev)
{
struct moxart_mac_priv_t *priv = netdev_priv(ndev);
@@ -168,6 +194,9 @@ static int moxart_mac_open(struct net_device *ndev)
moxart_update_mac_address(ndev);
moxart_mac_setup_desc_ring(ndev);
moxart_mac_enable(ndev);
+
+ phy_start(priv->phy_dev);
+
netif_start_queue(ndev);
netdev_dbg(ndev, "%s: IMR=0x%x, MACCR=0x%x\n",
@@ -183,6 +212,8 @@ static int moxart_mac_stop(struct net_device *ndev)
napi_disable(&priv->napi);
+ phy_stop(priv->phy_dev);
+
netif_stop_queue(ndev);
/* disable all interrupts */
@@ -435,12 +466,49 @@ static struct net_device_ops moxart_netdev_ops = {
.ndo_set_mac_address = moxart_set_mac_address,
.ndo_validate_addr = eth_validate_addr,
.ndo_change_mtu = eth_change_mtu,
+ .ndo_do_ioctl = moxart_do_ioctl,
};
+static void moxart_adjust_link(struct net_device *ndev)
+{
+ struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+ unsigned long flags;
+ int status_change = 0;
+
+ if (priv->phy_dev->link) {
+ if (priv->speed != priv->phy_dev->speed) {
+ priv->speed = priv->phy_dev->speed;
+ status_change = 1;
+ }
+
+ if (priv->duplex != priv->phy_dev->duplex) {
+ spin_lock_irqsave(&priv->txlock, flags);
+
+ priv->duplex = priv->phy_dev->duplex;
+ moxart_mac_update_duplex(ndev);
+
+ spin_unlock_irqrestore(&priv->txlock, flags);
+ status_change = 1;
+ }
+ }
+
+ if (priv->link != priv->phy_dev->link) {
+ if (!priv->phy_dev->link) {
+ priv->speed = 0;
+ priv->duplex = -1;
+ }
+ priv->link = priv->phy_dev->link;
+ status_change = 1;
+ }
+
+ if (status_change)
+ phy_print_status(priv->phy_dev);
+}
+
static int moxart_mac_probe(struct platform_device *pdev)
{
struct device *p_dev = &pdev->dev;
- struct device_node *node = p_dev->of_node;
+ struct device_node *node = p_dev->of_node, *phy_node;
struct net_device *ndev;
struct moxart_mac_priv_t *priv;
struct resource *res;
@@ -461,6 +529,28 @@ static int moxart_mac_probe(struct platform_device *pdev)
priv = netdev_priv(ndev);
priv->ndev = ndev;
+ priv->link = 0;
+ priv->speed = 0;
+ priv->duplex = -1;
+
+ phy_node = of_parse_phandle(node, "phy-handle", 0);
+ if (!phy_node) {
+ dev_err(p_dev, "of_parse_phandle failed\n");
+ ret = -ENODEV;
+ goto init_fail;
+ }
+
+ if (phy_node) {
+ priv->phy_dev = of_phy_connect(priv->ndev, phy_node,
+ &moxart_adjust_link,
+ 0, of_get_phy_mode(node));
+ if (!priv->phy_dev) {
+ dev_err(p_dev, "of_phy_connect failed\n");
+ ret = -ENODEV;
+ goto init_fail;
+ }
+ }
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
ndev->base_addr = res->start;
priv->base = devm_ioremap_resource(p_dev, res);
diff --git a/drivers/net/ethernet/moxa/moxart_ether.h b/drivers/net/ethernet/moxa/moxart_ether.h
index 2be9280..b8877bf 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.h
+++ b/drivers/net/ethernet/moxa/moxart_ether.h
@@ -297,6 +297,8 @@ struct moxart_mac_priv_t {
unsigned int reg_imr;
struct napi_struct napi;
struct net_device *ndev;
+ struct phy_device *phy_dev;
+ int speed, duplex, link;
dma_addr_t rx_base;
dma_addr_t rx_mapping[RX_DESC_NUM];
--
1.8.2.1
^ permalink raw reply related
* [PATCH v3 2/7] net: moxa: fix build_skb() memory corruption
From: Jonas Jensen @ 2014-01-20 11:13 UTC (permalink / raw)
To: netdev
Cc: linux-arm-kernel, linux-kernel, davem, f.fainelli, bhutchings,
Jonas Jensen
In-Reply-To: <1390216399-27028-1-git-send-email-jonas.jensen@gmail.com>
DMA buffer memory must be synchronized and copied before passing skb to
napi_gro_receive(). The use of build_skb() can lead to memory corruption,
replace it with netdev_alloc_skb_ip_align() and memcpy().
Addresses https://bugzilla.kernel.org/show_bug.cgi?id=69041
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
This fixes the following error on wget download (or ncftp),
usually after only a few seconds:
"read error: Bad address"
On receiving this error, wget exits. The download is not resumed (busybox default).
Applies to next-20140120
drivers/net/ethernet/moxa/moxart_ether.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index aa45607..17c9f0e 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -226,14 +226,21 @@ static int moxart_rx_poll(struct napi_struct *napi, int budget)
if (len > RX_BUF_SIZE)
len = RX_BUF_SIZE;
- skb = build_skb(priv->rx_buf[rx_head], priv->rx_buf_size);
+ dma_sync_single_for_cpu(&ndev->dev,
+ priv->rx_mapping[rx_head],
+ priv->rx_buf_size, DMA_FROM_DEVICE);
+ skb = netdev_alloc_skb_ip_align(ndev, len);
if (unlikely(!skb)) {
- net_dbg_ratelimited("build_skb failed\n");
+ net_dbg_ratelimited("netdev_alloc_skb_ip_align failed\n");
priv->stats.rx_dropped++;
priv->stats.rx_errors++;
}
-
+ memcpy(skb->data, priv->rx_buf[rx_head], len);
skb_put(skb, len);
+ dma_sync_single_for_device(&ndev->dev,
+ priv->rx_mapping[rx_head],
+ priv->rx_buf_size, DMA_FROM_DEVICE);
+
skb->protocol = eth_type_trans(skb, ndev);
napi_gro_receive(&priv->napi, skb);
rx++;
--
1.8.2.1
^ permalink raw reply related
* [PATCH v3 1/7] net: moxa: clear TX descriptor length bits between sends
From: Jonas Jensen @ 2014-01-20 11:13 UTC (permalink / raw)
To: netdev
Cc: linux-arm-kernel, linux-kernel, davem, f.fainelli, bhutchings,
Jonas Jensen
Add TX_DESC1_BUF_SIZE_MASK to bits that are cleared, before the TX buffer
length is set. Failing to do so can cause the controller to drop dead
i.e. all TX interrupts stop, resulting in complete communication failure.
Addresses https://bugzilla.kernel.org/show_bug.cgi?id=69031
Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---
Notes:
Applies to next-20140120
drivers/net/ethernet/moxa/moxart_ether.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index 5020fd4..aa45607 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -348,7 +348,8 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
txdes1 = readl(desc + TX_REG_OFFSET_DESC1);
txdes1 |= TX_DESC1_LTS | TX_DESC1_FTS;
- txdes1 &= ~(TX_DESC1_FIFO_COMPLETE | TX_DESC1_INTR_COMPLETE);
+ txdes1 &= ~(TX_DESC1_FIFO_COMPLETE | TX_DESC1_INTR_COMPLETE |
+ TX_DESC1_BUF_SIZE_MASK);
txdes1 |= (len & TX_DESC1_BUF_SIZE_MASK);
writel(txdes1, desc + TX_REG_OFFSET_DESC1);
writel(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
--
1.8.2.1
^ permalink raw reply related
* [patch] rxrpc: out of bound read in debug code
From: Dan Carpenter @ 2014-01-20 10:28 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, kernel-janitors
Smatch complains because we are using an untrusted index into the
rxrpc_acks[] array. It's just a read and it's only in the debug code,
but it's simple enough to add a check and fix it.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index e4d9cbcff402..cd97a0ce48d8 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -21,10 +21,17 @@
static unsigned int rxrpc_ack_defer = 1;
-static const char *const rxrpc_acks[] = {
- "---", "REQ", "DUP", "OOS", "WIN", "MEM", "PNG", "PNR", "DLY", "IDL",
- "-?-"
-};
+static const char *rxrpc_acks(u8 reason)
+{
+ static const char *const str[] = {
+ "---", "REQ", "DUP", "OOS", "WIN", "MEM", "PNG", "PNR", "DLY",
+ "IDL", "-?-"
+ };
+
+ if (reason >= ARRAY_SIZE(str))
+ reason = ARRAY_SIZE(str) - 1;
+ return str[reason];
+}
static const s8 rxrpc_ack_priority[] = {
[0] = 0,
@@ -50,7 +57,7 @@ void __rxrpc_propose_ACK(struct rxrpc_call *call, u8 ack_reason,
ASSERTCMP(prior, >, 0);
_enter("{%d},%s,%%%x,%u",
- call->debug_id, rxrpc_acks[ack_reason], ntohl(serial),
+ call->debug_id, rxrpc_acks(ack_reason), ntohl(serial),
immediate);
if (prior < rxrpc_ack_priority[call->ackr_reason]) {
@@ -637,7 +644,7 @@ process_further:
hard,
ntohl(ack.previousPacket),
ntohl(ack.serial),
- rxrpc_acks[ack.reason],
+ rxrpc_acks(ack.reason),
ack.nAcks);
rxrpc_extract_ackinfo(call, skb, latest, ack.nAcks);
@@ -1180,7 +1187,7 @@ send_ACK:
ntohl(ack.firstPacket),
ntohl(ack.previousPacket),
ntohl(ack.serial),
- rxrpc_acks[ack.reason],
+ rxrpc_acks(ack.reason),
ack.nAcks);
del_timer_sync(&call->ack_timer);
^ permalink raw reply related
* [PATCH] 8021q: update description
From: yegorslists @ 2014-01-20 9:49 UTC (permalink / raw)
To: netdev; +Cc: davem, Yegor Yefremov
From: Yegor Yefremov <yegorslists@googlemail.com>
Replace deprecated 'vconfig' tool with 'ip' from 'iproute2'. Add
some beautifications like replacing 'ethernet' with 'Ethernet' and
removing unneeded spaces.
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
net/8021q/Kconfig | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/8021q/Kconfig b/net/8021q/Kconfig
index b85a91f..4232018 100644
--- a/net/8021q/Kconfig
+++ b/net/8021q/Kconfig
@@ -6,11 +6,11 @@ config VLAN_8021Q
tristate "802.1Q/802.1ad VLAN Support"
---help---
Select this and you will be able to create 802.1Q VLAN interfaces
- on your ethernet interfaces. 802.1Q VLAN supports almost
- everything a regular ethernet interface does, including
- firewalling, bridging, and of course IP traffic. You will need
- the 'vconfig' tool from the VLAN project in order to effectively
- use VLANs. See the VLAN web page for more information:
+ on your Ethernet interfaces. 802.1Q VLAN supports almost
+ everything a regular Ethernet interface does, including
+ firewalling, bridging, and of course IP traffic. You will need
+ the 'ip' utility in order to effectively use VLANs.
+ See the VLAN web page for more information:
<http://www.candelatech.com/~greear/vlan.html>
To compile this code as a module, choose M here: the module
--
1.7.7
^ permalink raw reply related
* Re: [PATCH net] tun: handle copy failure in tun_put_user()
From: Michael S. Tsirkin @ 2014-01-20 9:52 UTC (permalink / raw)
To: Jason Wang
Cc: David Miller, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w,
linux-man-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <52DCED12.501-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Mon, Jan 20, 2014 at 05:32:02PM +0800, Jason Wang wrote:
> On 01/20/2014 04:43 PM, Michael S. Tsirkin wrote:
> > On Sun, Jan 19, 2014 at 07:48:56PM -0800, David Miller wrote:
> >> From: Jason Wang <jasowang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >> Date: Mon, 20 Jan 2014 11:16:48 +0800
> >>
> >>> This patch return the error code of copy helpers in tun_put_user() instead of
> >>> ignoring them.
> >>>
> >>> Cc: Michael S. Tsirkin <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >>> Signed-off-by: Jason Wang <jasowang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > I'm not sure we need to worry about this too much.
> > But if yes, a bunch of places besides tun should be
> > changed.
>
> Yes, I send the patch because the error processing here is different
> from what macvtap does. Macvtap just return error in this case and so do
> packet socket.
I suspect we just need to document that invalid address simply results
in unspecified behaviour. We try to return EFAULT to help debugging
sometimes but it's on a best effort basis.
>From this point of view EFAULT seems easier to debug than truncating the packet.
In any case even if we change Linux - applications won't be able to rely
on this for a long while.
So maybe we shouldn't do anything.
> > Consider for example udp_recvmsg: it
> > never seems to return any error except -EAGAIN.
> >
> > Is this a bug? Man page for recvmsg says:
> > EFAULT The receive buffer pointer(s) point outside the process's address
> > space.
> >
> > this isn't very clear: does this mean "all pointers are invalid"
> > or "some pointers are invalid"?
> > Also, what if pointers themselves are valid but length
> > makes us go outside the address space?
> >
> > I'm guessing the simplest way is to clarify in the man page that
> > passing invalid pointers / lengths is not guaranteed
> > to result in EFAULT and that Linux makes no guarantees
> > about the returned length in this case.
> >
> > Cc linux-man in case they can suggest some insights on this.
> >
> >> If you perform some of the copy successfully, you have to report that
> >> length rather than just an error.
> >>
> >> Otherwise userland has no way to determine how much of the data was
> >> successfully sourced.
> >>
> >> I'm not applying this, sorry.
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net] tun: handle copy failure in tun_put_user()
From: Jason Wang @ 2014-01-20 9:32 UTC (permalink / raw)
To: Michael S. Tsirkin, David Miller
Cc: netdev, linux-kernel, mtk.manpages, linux-man
In-Reply-To: <20140120084349.GA7677@redhat.com>
On 01/20/2014 04:43 PM, Michael S. Tsirkin wrote:
> On Sun, Jan 19, 2014 at 07:48:56PM -0800, David Miller wrote:
>> From: Jason Wang <jasowang@redhat.com>
>> Date: Mon, 20 Jan 2014 11:16:48 +0800
>>
>>> This patch return the error code of copy helpers in tun_put_user() instead of
>>> ignoring them.
>>>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> I'm not sure we need to worry about this too much.
> But if yes, a bunch of places besides tun should be
> changed.
Yes, I send the patch because the error processing here is different
from what macvtap does. Macvtap just return error in this case and so do
packet socket.
> Consider for example udp_recvmsg: it
> never seems to return any error except -EAGAIN.
>
> Is this a bug? Man page for recvmsg says:
> EFAULT The receive buffer pointer(s) point outside the process's address
> space.
>
> this isn't very clear: does this mean "all pointers are invalid"
> or "some pointers are invalid"?
> Also, what if pointers themselves are valid but length
> makes us go outside the address space?
>
> I'm guessing the simplest way is to clarify in the man page that
> passing invalid pointers / lengths is not guaranteed
> to result in EFAULT and that Linux makes no guarantees
> about the returned length in this case.
>
> Cc linux-man in case they can suggest some insights on this.
>
>> If you perform some of the copy successfully, you have to report that
>> length rather than just an error.
>>
>> Otherwise userland has no way to determine how much of the data was
>> successfully sourced.
>>
>> I'm not applying this, sorry.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH v5] can: add Renesas R-Car CAN driver
From: Marc Kleine-Budde @ 2014-01-20 9:18 UTC (permalink / raw)
To: Sergei Shtylyov, netdev, wg, linux-can; +Cc: linux-sh, vksavl
In-Reply-To: <201312270037.15822.sergei.shtylyov@cogentembedded.com>
[-- Attachment #1: Type: text/plain, Size: 37958 bytes --]
On 12/26/2013 10:37 PM, Sergei Shtylyov wrote:
> Add support for the CAN controller found in Renesas R-Car SoCs.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> The patch is against the 'linux-can-next.git' repo.
>
> Changes in version 5:
> - removed the code specific to the mailbox mode handling and added the code
> for the FIFO mode, dropping support for CAN_CTRLMODE_ONE_SHOT;
> - stop accumulating errors in the byte 1 of an error frame;
> - added #define RCAR_CAN_CTLR_CANM for CTLR.CANM field, added 'CANM_' infix to
> all its values;
> - replaced ~RCAR_CAN_CTLR_CANM_FORCE_RESET with ~RCAR_CAN_CTLR_CANM;
> - added polling for the CAN reset status bit when going from/to CAN reset.
>
> Changes in version 4:
> - added 'RCAR_CAN_' prefix to all #define's;
> - replaced all BIT(N) invocations with (1 << N) which allowed to remove casts
> to 'u8';
> - called rcar_can_set_bittiming() from rcar_can_start() again and stopped
> registering it as do_set_bittiming() method which allowed to remove clock
> API calls and control register writes from this function and make it *void*;
> - added #define RCAR_CAN_CTLR_IDFM for more clarity;
> - clarified comment to #define RCAR_CAN_IER_TXMIE;
> - did some whitespace cleanups.
>
> Changes in version 3:
> - replaced the register #define's with 'struct rcar_can_regs' fields, replaced
> rcar_can_{read|write}[bwl]() with mere {read|write}[bwl]();
> - removed hardware bus-off recovery support which allowed to also remove
> rcar_can_start() prototype;
> - added RX/TX error count to error data frame for error warning/passive;
> - moved TX completion interrupt handling into separate function;
> - added '__packed' to 'struct rcar_can_mbox_regs' and 'struct rcar_can_regs';
> - removed unneeded type cast in the probe() method.
>
> Changes in version 2:
> - added function to clean up TX mailboxes after bus error and bus-off;
> - added module parameter to enable hardware recovery from bus-off, added handler
> for the bus-off recovery interrupt, and set the control register according to
> the parameter value and the restart timer setting in rcar_can_start();
> - changed the way CAN_ERR_CRTL_[RT]X_{PASSIVE|WARNING} flags are set to a more
> realistic one;
> - replaced MBX_* macros and rcar_can_mbx_{read|write}[bl]() functions with
> 'struct rcar_can_mbox_regs', 'struct rcar_can_regs', and {read|write}[bl](),
> replaced 'reg_base' field of 'struct rcar_can_priv' with 'struct rcar_can_regs
> __iomem *regs';
> - added 'ier' field to 'struct rcar_can_priv' to cache the current value of the
> interrupt enable register;
> - added a check for enabled interrupts on entry to rcar_can_interrupt();
> - limited transmit mailbox search loop in rcar_can_interrupt();
> - decoupled TX byte count increment from can_get_echo_skb() call;
> - removed netif_queue_stopped() call from rcar_can_interrupt();
> - added clk_prepare_enable()/clk_disable_unprepare() to ndo_{open|close}(),
> do_set_bittiming(), and do_get_berr_counter() methods, removed clk_enable()
> call from the probe() method and clk_disable() call from the remove() method;
> - allowed rcar_can_set_bittiming() to be called when the device is closed and
> remove explicit call to it from rcar_can_start();
> - switched to using mailbox number priority transmit mode, and switched to the
> sequential mailbox use in ndo_start_xmit() method;
> - stopped reading the message control registers in ndo_start_xmit() method;
> - avoided returning NETDEV_TX_BUSY from ndo_start_xmit() method;
> - stopped reading data when RTR bit is set in the CAN frame;
> - made 'num_pkts' variable *int* and moved its check to the *while* condition in
> rcar_can_rx_poll();
> - used dev_get_platdata() in the probe() method;
> - enabled bus error interrupt only if CAN_CTRLMODE_BERR_REPORTING flag is set;
> - started reporting CAN_CTRLMODE_BERR_REPORTING support and stopped reporting
> CAN_CTRLMODE_3_SAMPLES support;
> - set CAN_ERR_ACK flag on ACK error;
> - switched to incrementing bus error counter only once per bus error interrupt;
> - started switching to CAN sleep mode in rcar_can_stop() and stopped switching
> to it in the remove() method;
> - removed netdev_err() calls on allocation failure in rcar_can_error() and
> rcar_can_rx_pkt();
> - removed "CANi" from the register offset macro comments.
>
> drivers/net/can/Kconfig | 9
> drivers/net/can/Makefile | 1
> drivers/net/can/rcar_can.c | 857 ++++++++++++++++++++++++++++++++++
> include/linux/can/platform/rcar_can.h | 15
> 4 files changed, 882 insertions(+)
>
> Index: linux-can-next/drivers/net/can/Kconfig
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Kconfig
> +++ linux-can-next/drivers/net/can/Kconfig
> @@ -125,6 +125,15 @@ config CAN_GRCAN
> endian syntheses of the cores would need some modifications on
> the hardware level to work.
>
> +config CAN_RCAR
> + tristate "Renesas R-Car CAN controller"
> + ---help---
> + Say Y here if you want to use CAN controller found on Renesas R-Car
> + SoCs.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called rcar_can.
> +
> source "drivers/net/can/mscan/Kconfig"
>
> source "drivers/net/can/sja1000/Kconfig"
> Index: linux-can-next/drivers/net/can/Makefile
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Makefile
> +++ linux-can-next/drivers/net/can/Makefile
> @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3) += janz-ica
> obj-$(CONFIG_CAN_FLEXCAN) += flexcan.o
> obj-$(CONFIG_PCH_CAN) += pch_can.o
> obj-$(CONFIG_CAN_GRCAN) += grcan.o
> +obj-$(CONFIG_CAN_RCAR) += rcar_can.o
>
> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> Index: linux-can-next/drivers/net/can/rcar_can.c
> ===================================================================
> --- /dev/null
> +++ linux-can-next/drivers/net/can/rcar_can.c
> @@ -0,0 +1,857 @@
> +/*
> + * Renesas R-Car CAN device driver
> + *
> + * Copyright (C) 2013 Cogent Embedded, Inc. <source@cogentembedded.com>
> + * Copyright (C) 2013 Renesas Solutions Corp.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/errno.h>
> +#include <linux/netdevice.h>
> +#include <linux/platform_device.h>
> +#include <linux/can/led.h>
> +#include <linux/can/dev.h>
> +#include <linux/clk.h>
> +#include <linux/can/platform/rcar_can.h>
> +
> +#define RCAR_CAN_DRV_NAME "rcar_can"
> +
> +/* Mailbox configuration:
> + * mailbox 60 - 63 - Rx FIFO mailboxes
> + * mailbox 56 - 59 - Tx FIFO mailboxes
> + * non-FIFO mailboxes are not used
> + */
> +#define RCAR_CAN_N_MBX 64 /* Number of mailboxes in non-FIFO mode */
> +#define RCAR_CAN_RX_FIFO_MBX 60 /* Mailbox - window to Rx FIFO */
> +#define RCAR_CAN_TX_FIFO_MBX 56 /* Mailbox - window to Tx FIFO */
> +#define RCAR_CAN_N_ECHO_SKB 10
> +
> +/* Mailbox registers structure */
> +struct rcar_can_mbox_regs {
> + u32 id; /* IDE and RTR bits, SID and EID */
> + u8 stub; /* Not used */
> + u8 dlc; /* Data Length Code - bits [0..3] */
> + u8 data[8]; /* Data Bytes */
> + u8 tsh; /* Time Stamp Higher Byte */
> + u8 tsl; /* Time Stamp Lower Byte */
> +} __packed;
If you have contact to the hardware designer please blame him for
placing the data register unaligned into the register space. :)
> +
> +struct rcar_can_regs {
> + struct rcar_can_mbox_regs mb[RCAR_CAN_N_MBX]; /* Mailbox registers */
> + u32 mkr_2_9[8]; /* Mask Registers 2-9 */
> + u32 fidcr[2]; /* FIFO Received ID Compare Register */
> + u32 mkivlr1; /* Mask Invalid Register 1 */
> + u32 mier1; /* Mailbox Interrupt Enable Register 1 */
> + u32 mkr_0_1[2]; /* Mask Registers 0-1 */
> + u32 mkivlr0; /* Mask Invalid Register 0*/
> + u32 mier0; /* Mailbox Interrupt Enable Register 0 */
> + u8 pad_440[0x3c0];
> + u8 mctl[64]; /* Message Control Registers */
> + u16 ctlr; /* Control Register */
> + u16 str; /* Status register */
> + u8 bcr[3]; /* Bit Configuration Register */
> + u8 clkr; /* Clock Select Register */
bcr[3] and clkr looks broken, see comment where the register is used.
> + u8 rfcr; /* Receive FIFO Control Register */
> + u8 rfpcr; /* Receive FIFO Pointer Control Register */
> + u8 tfcr; /* Transmit FIFO Control Register */
> + u8 tfpcr; /* Transmit FIFO Pointer Control Register */
> + u8 eier; /* Error Interrupt Enable Register */
> + u8 eifr; /* Error Interrupt Factor Judge Register */
> + u8 recr; /* Receive Error Count Register */
> + u8 tecr; /* Transmit Error Count Register */
> + u8 ecsr; /* Error Code Store Register */
> + u8 cssr; /* Channel Search Support Register */
> + u8 mssr; /* Mailbox Search Status Register */
> + u8 msmr; /* Mailbox Search Mode Register */
> + u16 tsr; /* Time Stamp Register */
> + u8 afsr; /* Acceptance Filter Support Register */
> + u8 pad_857;
> + u8 tcr; /* Test Control Register */
> + u8 pad_859[7];
> + u8 ier; /* Interrupt Enable Register */
> + u8 isr; /* Interrupt Status Register */
> + u8 pad_862;
> + u8 mbsmr; /* Mailbox Search Mask Register */
> +} __packed;
> +
> +struct rcar_can_priv {
> + struct can_priv can; /* Must be the first member! */
> + struct net_device *ndev;
> + struct napi_struct napi;
> + struct rcar_can_regs __iomem *regs;
> + struct clk *clk;
> + spinlock_t skb_lock;
> + u32 frames_queued;
> + u32 bytes_queued;
> + u8 clock_select;
> + u8 ier;
> +};
> +
> +static const struct can_bittiming_const rcar_can_bittiming_const = {
> + .name = RCAR_CAN_DRV_NAME,
> + .tseg1_min = 4,
> + .tseg1_max = 16,
> + .tseg2_min = 2,
> + .tseg2_max = 8,
> + .sjw_max = 4,
> + .brp_min = 1,
> + .brp_max = 1024,
> + .brp_inc = 1,
> +};
> +
> +/* Control Register bits */
> +#define RCAR_CAN_CTLR_BOM (3 << 11) /* Bus-Off Recovery Mode Bits */
> +#define RCAR_CAN_CTLR_BOM_ENT (1 << 11) /* Entry to halt mode */
> + /* at bus-off entry */
> +#define RCAR_CAN_CTLR_SLPM (1 << 10)
> +#define RCAR_CAN_CTLR_CANM (3 << 8) /* Operating Mode Select Bit */
> +#define RCAR_CAN_CTLR_CANM_HALT (1 << 9)
> +#define RCAR_CAN_CTLR_CANM_RESET (1 << 8)
> +#define RCAR_CAN_CTLR_CANM_FORCE_RESET (3 << 8)
> +#define RCAR_CAN_CTLR_MLM (1 << 3) /* Message Lost Mode Select */
> +#define RCAR_CAN_CTLR_IDFM (3 << 1) /* ID Format Mode Select Bits */
> +#define RCAR_CAN_CTLR_IDFM_MIXED (1 << 2) /* Mixed ID mode */
> +#define RCAR_CAN_CTLR_MBM (1 << 0) /* Mailbox Mode select */
> +
> +/* Status Register bits */
> +#define RCAR_CAN_STR_RSTST (1 << 8) /* Reset Status Bit */
> +
> +/* FIFO Received ID Compare Registers 0 and 1 bits */
> +#define RCAR_CAN_FIDCR_IDE (1 << 31) /* ID Extension Bit */
> +#define RCAR_CAN_FIDCR_RTR (1 << 30) /* Remote Transmission Request Bit */
> +
> +/* Receive FIFO Control Register bits */
> +#define RCAR_CAN_RFCR_RFEST (1 << 7) /* Receive FIFO Empty Status Flag */
> +#define RCAR_CAN_RFCR_RFUST (7 << 1) /* Receive FIFO Unread Message */
> + /* Number Status Bits */
> +#define RCAR_CAN_RFCR_RFUST_SHIFT 1 /* Offset of Receive FIFO Unread */
> + /* Message Number Status Bits */
> +#define RCAR_CAN_RFCR_RFE (1 << 0) /* Receive FIFO Enable */
> +
> +/* Transmit FIFO Control Register bits */
> +#define RCAR_CAN_TFCR_TFUST (7 << 1) /* Transmit FIFO Unsent Message */
> + /* Number Status Bits */
> +#define RCAR_CAN_TFCR_TFUST_SHIFT 1 /* Offset of Transmit FIFO Unsent */
> + /* Message Number Status Bits */
> +#define RCAR_CAN_TFCR_TFE (1 << 0) /* Transmit FIFO Enable */
> +
> +#define RCAR_CAN_N_RX_MKREGS1 2 /* Number of mask registers */
> + /* for Rx mailboxes 0-31 */
> +#define RCAR_CAN_N_RX_MKREGS2 8
> +
> +/* Bit Configuration Register settings */
> +#define RCAR_CAN_BCR_TSEG1(x) (((x) & 0x0f) << 28)
> +#define RCAR_CAN_BCR_BPR(x) (((x) & 0x3ff) << 16)
> +#define RCAR_CAN_BCR_SJW(x) (((x) & 0x3) << 12)
> +#define RCAR_CAN_BCR_TSEG2(x) (((x) & 0x07) << 8)
> +
> +/* Mailbox and Mask Registers bits */
> +#define RCAR_CAN_IDE (1 << 31)
> +#define RCAR_CAN_RTR (1 << 30)
> +#define RCAR_CAN_SID_SHIFT 18
> +
> +/* Mailbox Interrupt Enable Register 1 bits */
> +#define RCAR_CAN_MIER1_RXFIE (1 << 28) /* Receive FIFO Interrupt Enable */
> +#define RCAR_CAN_MIER1_TXFIT (1 << 25) /* Transmit FIFO Interrupt Timing */
> + /* Generate interrupt when */
> + /* Tx FIFO becomes empty due to */
> + /* completion of transmission */
> +#define RCAR_CAN_MIER1_TXFIE (1 << 24) /* Transmit FIFO Interrupt Enable */
> +
> +/* Interrupt Enable Register bits */
> +#define RCAR_CAN_IER_ERSIE (1 << 5) /* Error (ERS) Interrupt Enable Bit */
> +#define RCAR_CAN_IER_RXFIE (1 << 4) /* Reception FIFO Interrupt */
> + /* Enable Bit */
> +#define RCAR_CAN_IER_TXFIE (1 << 3) /* Transmission FIFO Interrupt */
> + /* Enable Bit */
> +/* Interrupt Status Register bits */
> +#define RCAR_CAN_ISR_ERSF (1 << 5) /* Error (ERS) Interrupt Status Bit */
> +#define RCAR_CAN_ISR_RXFF (1 << 4) /* Reception FIFO Interrupt */
> + /* Status Bit */
> +#define RCAR_CAN_ISR_TXFF (1 << 3) /* Transmission FIFO Interrupt */
> + /* Status Bit */
> +
> +/* Error Interrupt Enable Register bits */
> +#define RCAR_CAN_EIER_BLIE (1 << 7) /* Bus Lock Interrupt Enable */
> +#define RCAR_CAN_EIER_OLIE (1 << 6) /* Overload Frame Transmit */
> + /* Interrupt Enable */
> +#define RCAR_CAN_EIER_ORIE (1 << 5) /* Receive Overrun Interrupt Enable */
> +#define RCAR_CAN_EIER_BORIE (1 << 4) /* Bus-Off Recovery Interrupt Enable */
> +#define RCAR_CAN_EIER_BOEIE (1 << 3) /* Bus-Off Entry Interrupt Enable */
> +#define RCAR_CAN_EIER_EPIE (1 << 2) /* Error Passive Interrupt Enable */
> +#define RCAR_CAN_EIER_EWIE (1 << 1) /* Error Warning Interrupt Enable */
> +#define RCAR_CAN_EIER_BEIE (1 << 0) /* Bus Error Interrupt Enable */
> +
> +/* Error Interrupt Factor Judge Register bits */
> +#define RCAR_CAN_EIFR_BLIF (1 << 7) /* Bus Lock Detect Flag */
> +#define RCAR_CAN_EIFR_OLIF (1 << 6) /* Overload Frame Transmission */
> + /* Detect Flag */
> +#define RCAR_CAN_EIFR_ORIF (1 << 5) /* Receive Overrun Detect Flag */
> +#define RCAR_CAN_EIFR_BORIF (1 << 4) /* Bus-Off Recovery Detect Flag */
> +#define RCAR_CAN_EIFR_BOEIF (1 << 3) /* Bus-Off Entry Detect Flag */
> +#define RCAR_CAN_EIFR_EPIF (1 << 2) /* Error Passive Detect Flag */
> +#define RCAR_CAN_EIFR_EWIF (1 << 1) /* Error Warning Detect Flag */
> +#define RCAR_CAN_EIFR_BEIF (1 << 0) /* Bus Error Detect Flag */
> +
> +/* Error Code Store Register bits */
> +#define RCAR_CAN_ECSR_EDPM (1 << 7) /* Error Display Mode Select Bit */
> +#define RCAR_CAN_ECSR_ADEF (1 << 6) /* ACK Delimiter Error Flag */
> +#define RCAR_CAN_ECSR_BE0F (1 << 5) /* Bit Error (dominant) Flag */
> +#define RCAR_CAN_ECSR_BE1F (1 << 4) /* Bit Error (recessive) Flag */
> +#define RCAR_CAN_ECSR_CEF (1 << 3) /* CRC Error Flag */
> +#define RCAR_CAN_ECSR_AEF (1 << 2) /* ACK Error Flag */
> +#define RCAR_CAN_ECSR_FEF (1 << 1) /* Form Error Flag */
> +#define RCAR_CAN_ECSR_SEF (1 << 0) /* Stuff Error Flag */
> +
> +#define RCAR_CAN_NAPI_WEIGHT 4
> +
> +static void tx_failure_cleanup(struct net_device *ndev)
> +{
> + u32 i;
> +
> + for (i = 0; i < RCAR_CAN_N_ECHO_SKB; i++)
> + can_free_echo_skb(ndev, i);
> +}
> +
> +static void rcar_can_error(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + struct net_device_stats *stats = &ndev->stats;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + u8 eifr, txerr = 0, rxerr = 0;
> +
> + /* Propagate the error condition to the CAN stack */
> + skb = alloc_can_err_skb(ndev, &cf);
> + if (!skb)
> + return;
> +
> + eifr = readb(&priv->regs->eifr);
> + if (eifr & (RCAR_CAN_EIFR_EWIF | RCAR_CAN_EIFR_EPIF)) {
> + cf->can_id |= CAN_ERR_CRTL;
> + txerr = readb(&priv->regs->tecr);
> + rxerr = readb(&priv->regs->recr);
> + cf->data[6] = txerr;
> + cf->data[7] = rxerr;
> + }
> + if (eifr & RCAR_CAN_EIFR_BEIF) {
> + int rx_errors = 0, tx_errors = 0;
> + u8 ecsr;
> +
> + netdev_dbg(priv->ndev, "Bus error interrupt:\n");
> + cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> + cf->data[2] = CAN_ERR_PROT_UNSPEC;
> +
> + ecsr = readb(&priv->regs->ecsr);
> + if (ecsr & RCAR_CAN_ECSR_ADEF) {
> + netdev_dbg(priv->ndev, "ACK Delimiter Error\n");
> + cf->data[3] |= CAN_ERR_PROT_LOC_ACK_DEL;
> + tx_errors++;
> + writeb(~RCAR_CAN_ECSR_ADEF, &priv->regs->ecsr);
> + }
> + if (ecsr & RCAR_CAN_ECSR_BE0F) {
> + netdev_dbg(priv->ndev, "Bit Error (dominant)\n");
> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> + tx_errors++;
> + writeb(~RCAR_CAN_ECSR_BE0F, &priv->regs->ecsr);
> + }
> + if (ecsr & RCAR_CAN_ECSR_BE1F) {
> + netdev_dbg(priv->ndev, "Bit Error (recessive)\n");
> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> + tx_errors++;
> + writeb(~RCAR_CAN_ECSR_BE1F, &priv->regs->ecsr);
> + }
> + if (ecsr & RCAR_CAN_ECSR_CEF) {
> + netdev_dbg(priv->ndev, "CRC Error\n");
> + cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
> + rx_errors++;
> + writeb(~RCAR_CAN_ECSR_CEF, &priv->regs->ecsr);
> + }
> + if (ecsr & RCAR_CAN_ECSR_AEF) {
> + netdev_dbg(priv->ndev, "ACK Error\n");
> + cf->can_id |= CAN_ERR_ACK;
> + cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
> + tx_errors++;
> + writeb(~RCAR_CAN_ECSR_AEF, &priv->regs->ecsr);
> + }
> + if (ecsr & RCAR_CAN_ECSR_FEF) {
> + netdev_dbg(priv->ndev, "Form Error\n");
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + rx_errors++;
> + writeb(~RCAR_CAN_ECSR_FEF, &priv->regs->ecsr);
> + }
> + if (ecsr & RCAR_CAN_ECSR_SEF) {
> + netdev_dbg(priv->ndev, "Stuff Error\n");
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> + rx_errors++;
> + writeb(~RCAR_CAN_ECSR_SEF, &priv->regs->ecsr);
> + }
> +
> + priv->can.can_stats.bus_error++;
> + ndev->stats.rx_errors += rx_errors;
> + ndev->stats.tx_errors += tx_errors;
> + writeb(~RCAR_CAN_EIFR_BEIF, &priv->regs->eifr);
> + }
> + if (eifr & RCAR_CAN_EIFR_EWIF) {
> + netdev_dbg(priv->ndev, "Error warning interrupt\n");
> + priv->can.state = CAN_STATE_ERROR_WARNING;
> + priv->can.can_stats.error_warning++;
> + cf->data[1] = txerr > rxerr ? CAN_ERR_CRTL_TX_WARNING :
> + CAN_ERR_CRTL_RX_WARNING;
> + /* Clear interrupt condition */
> + writeb(~RCAR_CAN_EIFR_EWIF, &priv->regs->eifr);
> + }
> + if (eifr & RCAR_CAN_EIFR_EPIF) {
> + netdev_dbg(priv->ndev, "Error passive interrupt\n");
> + priv->can.state = CAN_STATE_ERROR_PASSIVE;
> + priv->can.can_stats.error_passive++;
> + cf->data[1] = txerr > rxerr ? CAN_ERR_CRTL_TX_PASSIVE :
> + CAN_ERR_CRTL_RX_PASSIVE;
> + /* Clear interrupt condition */
> + writeb(~RCAR_CAN_EIFR_EPIF, &priv->regs->eifr);
> + }
> + if (eifr & RCAR_CAN_EIFR_BOEIF) {
> + netdev_dbg(priv->ndev, "Bus-off entry interrupt\n");
> + tx_failure_cleanup(ndev);
> + priv->ier = RCAR_CAN_IER_ERSIE;
> + writeb(priv->ier, &priv->regs->ier);
> + priv->can.state = CAN_STATE_BUS_OFF;
> + cf->can_id |= CAN_ERR_BUSOFF;
> + /* Clear interrupt condition */
> + writeb(~RCAR_CAN_EIFR_BOEIF, &priv->regs->eifr);
> + can_bus_off(ndev);
> + }
> + if (eifr & RCAR_CAN_EIFR_ORIF) {
> + netdev_dbg(priv->ndev, "Receive overrun error interrupt\n");
> + cf->can_id |= CAN_ERR_CRTL;
> + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> + ndev->stats.rx_over_errors++;
> + ndev->stats.rx_errors++;
> + writeb(~RCAR_CAN_EIFR_ORIF, &priv->regs->eifr);
> + }
> + if (eifr & RCAR_CAN_EIFR_OLIF) {
> + netdev_dbg(priv->ndev,
> + "Overload Frame Transmission error interrupt\n");
> + cf->can_id |= CAN_ERR_PROT;
> + cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
> + ndev->stats.rx_over_errors++;
> + ndev->stats.rx_errors++;
> + writeb(~RCAR_CAN_EIFR_OLIF, &priv->regs->eifr);
> + }
> +
> + netif_rx(skb);
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static void rcar_can_tx_done(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + struct net_device_stats *stats = &ndev->stats;
> + int i;
> +
> + spin_lock(&priv->skb_lock);
> + for (i = 0; i < priv->frames_queued; i++)
> + can_get_echo_skb(ndev, i);
> + stats->tx_bytes += priv->bytes_queued;
> + stats->tx_packets += priv->frames_queued;
> + priv->bytes_queued = 0;
> + priv->frames_queued = 0;
> + spin_unlock(&priv->skb_lock);
This looks broken. What happens if you send 2 CAN frames in a row, the
first one is send, a TX complete interrupt is issued and you handle it
here? You assume, that all CAN frames have been sent.
> + can_led_event(ndev, CAN_LED_EVENT_TX);
> + netif_wake_queue(ndev);
> +}
> +
> +static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
> +{
> + struct net_device *ndev = (struct net_device *)dev_id;
the cast is not needed
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + u8 isr;
> +
> + isr = readb(&priv->regs->isr);
> + if (!(isr & priv->ier))
> + return IRQ_NONE;
> +
> + if (isr & RCAR_CAN_ISR_ERSF)
> + rcar_can_error(ndev);
> +
> + if (isr & RCAR_CAN_ISR_TXFF) {
> + /* Clear interrupt */
> + writeb(isr & ~RCAR_CAN_ISR_TXFF, &priv->regs->isr);
> + rcar_can_tx_done(ndev);
> + }
> +
> + if (isr & RCAR_CAN_ISR_RXFF) {
> + if (napi_schedule_prep(&priv->napi)) {
> + /* Disable Rx FIFO interrupts */
> + priv->ier &= ~RCAR_CAN_IER_RXFIE;
> + writeb(priv->ier, &priv->regs->ier);
> + __napi_schedule(&priv->napi);
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void rcar_can_set_bittiming(struct net_device *dev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(dev);
> + struct can_bittiming *bt = &priv->can.bittiming;
> + u32 bcr;
> + u8 clkr;
> +
> + /* Don't overwrite CLKR with 32-bit BCR access */
> + /* CLKR has 8-bit access */
Can you explain the register layout here? Why do you access BCR with 32
bits when the register is defined as 3x8 bit? Can't you make it a
standard 32 bit register?
> + clkr = readb(&priv->regs->clkr);
> + bcr = RCAR_CAN_BCR_TSEG1(bt->phase_seg1 + bt->prop_seg - 1) |
> + RCAR_CAN_BCR_BPR(bt->brp - 1) | RCAR_CAN_BCR_SJW(bt->sjw - 1) |
> + RCAR_CAN_BCR_TSEG2(bt->phase_seg2 - 1);
> + writel(bcr, &priv->regs->bcr);
> + writeb(clkr, &priv->regs->clkr);
> +}
> +
> +static void rcar_can_start(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + u16 ctlr, str;
> +
> + /* Set controller to known mode:
> + * - FIFO mailbox mode
> + * - accept all messages
> + * - overrun mode
> + * CAN is in sleep mode after MCU hardware or software reset.
> + */
> + ctlr = readw(&priv->regs->ctlr);
> + ctlr &= ~RCAR_CAN_CTLR_SLPM;
> + writew(ctlr, &priv->regs->ctlr);
> + /* Go to reset mode */
> + ctlr |= RCAR_CAN_CTLR_CANM_FORCE_RESET;
> + writew(ctlr, &priv->regs->ctlr);
> + do {
> + str = readw(&priv->regs->str);
> + } while (!(str & RCAR_CAN_STR_RSTST));
Please add a timeout for this loop and the loop below.
> + rcar_can_set_bittiming(ndev);
> + ctlr |= RCAR_CAN_CTLR_IDFM_MIXED; /* Select mixed ID mode */
> + ctlr |= RCAR_CAN_CTLR_BOM_ENT; /* Entry to halt mode automatically */
> + /* at bus-off */
> + ctlr |= RCAR_CAN_CTLR_MBM; /* Select FIFO mailbox mode */
> + ctlr |= RCAR_CAN_CTLR_MLM; /* Overrun mode */
> + writew(ctlr, &priv->regs->ctlr);
> +
> + writeb(priv->clock_select, &priv->regs->clkr);
> +
> + /* Accept all SID and EID */
> + writel(0, &priv->regs->mkr_2_9[6]);
> + writel(0, &priv->regs->mkr_2_9[7]);
> + /* In FIFO mailbox mode, write "0" to bits 24 to 31 */
> + writel(0, &priv->regs->mkivlr1);
> + /* Accept all frames */
> + writel(0, &priv->regs->fidcr[0]);
> + writel(RCAR_CAN_FIDCR_IDE | RCAR_CAN_FIDCR_RTR, &priv->regs->fidcr[1]);
> + /* Enable and configure FIFO mailbox interrupts */
> + writel(RCAR_CAN_MIER1_RXFIE | RCAR_CAN_MIER1_TXFIT |
> + RCAR_CAN_MIER1_TXFIE, &priv->regs->mier1);
> +
> + priv->ier = RCAR_CAN_IER_ERSIE | RCAR_CAN_IER_RXFIE |
> + RCAR_CAN_IER_TXFIE;
> + writeb(priv->ier, &priv->regs->ier);
> +
> + /* Accumulate error codes */
> + writeb(RCAR_CAN_ECSR_EDPM, &priv->regs->ecsr);
> + /* Enable error interrupts */
> + writeb(RCAR_CAN_EIER_EWIE | RCAR_CAN_EIER_EPIE | RCAR_CAN_EIER_BOEIE |
> + (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING ?
> + RCAR_CAN_EIER_BEIE : 0) | RCAR_CAN_EIER_ORIE |
> + RCAR_CAN_EIER_OLIE, &priv->regs->eier);
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> + /* Go to operation mode */
> + writew(ctlr & ~RCAR_CAN_CTLR_CANM, &priv->regs->ctlr);
> + do {
> + str = readw(&priv->regs->str);
> + } while (str & RCAR_CAN_STR_RSTST);
> + /* Enable Rx and Tx FIFO */
> + writeb(RCAR_CAN_RFCR_RFE, &priv->regs->rfcr);
> + writeb(RCAR_CAN_TFCR_TFE, &priv->regs->tfcr);
> +}
> +
> +static int rcar_can_open(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + int err;
> +
> + clk_prepare_enable(priv->clk);
clk_prepare_enable can fail
> + err = open_candev(ndev);
> + if (err) {
> + netdev_err(ndev, "open_candev() failed %d\n", err);
> + goto out;
please adjust the jump label, you have to disable the clock.
> + }
> + napi_enable(&priv->napi);
> + err = request_irq(ndev->irq, rcar_can_interrupt, 0, ndev->name, ndev);
> + if (err) {
> + netdev_err(ndev, "error requesting interrupt %x\n", ndev->irq);
> + goto out_close;
> + }
> + can_led_event(ndev, CAN_LED_EVENT_OPEN);
> + rcar_can_start(ndev);
> + netif_start_queue(ndev);
> + return 0;
> +out_close:
> + napi_disable(&priv->napi);
> + close_candev(ndev);
> + clk_disable_unprepare(priv->clk);
> +out:
> + return err;
> +}
> +
> +static void rcar_can_stop(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + u16 ctlr, str;
> +
> + /* Go to (force) reset mode */
> + ctlr = readw(&priv->regs->ctlr);
> + ctlr |= RCAR_CAN_CTLR_CANM_FORCE_RESET;
> + writew(ctlr, &priv->regs->ctlr);
> + do {
> + str = readw(&priv->regs->str);
> + } while (!(str & RCAR_CAN_STR_RSTST));
please add a timeout to the loop
> + writel(0, &priv->regs->mier0);
> + writel(0, &priv->regs->mier1);
> + writeb(0, &priv->regs->ier);
> + writeb(0, &priv->regs->eier);
> + /* Go to sleep mode */
> + ctlr |= RCAR_CAN_CTLR_SLPM;
> + writew(ctlr, &priv->regs->ctlr);
> + priv->can.state = CAN_STATE_STOPPED;
> +}
> +
> +static int rcar_can_close(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> +
> + netif_stop_queue(ndev);
> + rcar_can_stop(ndev);
> + free_irq(ndev->irq, ndev);
> + napi_disable(&priv->napi);
> + clk_disable_unprepare(priv->clk);
> + close_candev(ndev);
> + can_led_event(ndev, CAN_LED_EVENT_STOP);
> + return 0;
> +}
> +
> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
> + struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + struct can_frame *cf = (struct can_frame *)skb->data;
> + u32 data, i;
> + unsigned long flags;
> + u8 tfcr;
> +
> + if (can_dropped_invalid_skb(ndev, skb))
> + return NETDEV_TX_OK;
> + tfcr = readb(&priv->regs->tfcr);
> + if ((tfcr & RCAR_CAN_TFCR_TFUST) >> RCAR_CAN_TFCR_TFUST_SHIFT > 2)
> + netif_stop_queue(ndev);
Can you explain what's checked here?
> +
> + if (cf->can_id & CAN_EFF_FLAG) {
> + /* Extended frame format */
> + data = (cf->can_id & CAN_EFF_MASK) | RCAR_CAN_IDE;
> + } else {
> + /* Standard frame format */
> + data = (cf->can_id & CAN_SFF_MASK) << RCAR_CAN_SID_SHIFT;
> + }
> + if (cf->can_id & CAN_RTR_FLAG) {
> + /* Remote transmission request */
> + data |= RCAR_CAN_RTR;
> + }
You can move the comments into the line of if and else and remove the {
& } as there is only one line after if and else.
> + writel(data, &priv->regs->mb[RCAR_CAN_TX_FIFO_MBX].id);
> +
> + writeb(cf->can_dlc, &priv->regs->mb[RCAR_CAN_TX_FIFO_MBX].dlc);
> +
> + for (i = 0; i < cf->can_dlc; i++)
> + writeb(cf->data[i],
> + &priv->regs->mb[RCAR_CAN_TX_FIFO_MBX].data[i]);
> +
> + spin_lock_irqsave(&priv->skb_lock, flags);
> + can_put_echo_skb(skb, ndev, priv->frames_queued++);
> + priv->bytes_queued += cf->can_dlc;
How does the frames_queued and bytes_queued mechanism work?
> + spin_unlock_irqrestore(&priv->skb_lock, flags);
> + /* Start Tx: write 0xFF to the TFPCR register to increment
> + * the CPU-side pointer for the transmit FIFO to the next
> + * mailbox location
> + */
> + writeb(0xFF, &priv->regs->tfpcr);
please use lowercase for hex.
> +
> + return NETDEV_TX_OK;
I'm missing flow control here. You have to stop the queue if there isn't
any room in the tx fifo.
> +}
> +
> +static const struct net_device_ops rcar_can_netdev_ops = {
> + .ndo_open = rcar_can_open,
> + .ndo_stop = rcar_can_close,
> + .ndo_start_xmit = rcar_can_start_xmit,
> +};
> +
> +static void rcar_can_rx_pkt(struct rcar_can_priv *priv)
> +{
> + struct net_device_stats *stats = &priv->ndev->stats;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + u32 data;
> + u8 dlc;
> +
> + skb = alloc_can_skb(priv->ndev, &cf);
> + if (!skb) {
> + stats->rx_dropped++;
> + return;
> + }
> +
> + data = readl(&priv->regs->mb[RCAR_CAN_RX_FIFO_MBX].id);
> + if (data & RCAR_CAN_IDE)
> + cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
> + else
> + cf->can_id = (data >> RCAR_CAN_SID_SHIFT) & CAN_SFF_MASK;
> +
> + dlc = readb(&priv->regs->mb[RCAR_CAN_RX_FIFO_MBX].dlc);
> + cf->can_dlc = get_can_dlc(dlc);
> + if (data & RCAR_CAN_RTR) {
> + cf->can_id |= CAN_RTR_FLAG;
> + } else {
> + for (dlc = 0; dlc < cf->can_dlc; dlc++)
> + cf->data[dlc] =
> + readb(&priv->regs->mb[RCAR_CAN_RX_FIFO_MBX].data[dlc]);
> + }
> +
> + can_led_event(priv->ndev, CAN_LED_EVENT_RX);
> +
> + netif_receive_skb(skb);
> + stats->rx_bytes += cf->can_dlc;
> + stats->rx_packets++;
> +}
> +
> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
> +{
> + struct rcar_can_priv *priv = container_of(napi,
> + struct rcar_can_priv, napi);
> + int num_pkts = 0;
> +
> + while (num_pkts < quota) {
> + u8 i, rfcr, nframes, isr;
> +
> + isr = readb(&priv->regs->isr);
> + /* Clear interrupt bit */
> + if (isr & RCAR_CAN_ISR_RXFF)
> + writeb(isr & ~RCAR_CAN_ISR_RXFF, &priv->regs->isr);
> + rfcr = readb(&priv->regs->rfcr);
> + if (rfcr & RCAR_CAN_RFCR_RFEST)
> + break;
> + nframes = (rfcr & RCAR_CAN_RFCR_RFUST) >>
> + RCAR_CAN_RFCR_RFUST_SHIFT;
> + for (i = 0; i < nframes; i++) {
> + rcar_can_rx_pkt(priv);
> + /* Write 0xFF to the RFPCR register to increment
> + * the CPU-side pointer for the receive FIFO
> + * to the next mailbox location
> + */
> + writeb(0xFF, &priv->regs->rfpcr);
> + ++num_pkts;
> + }
The for loop inside the while loop makes no sense if you increment
num_pkts. You are not allowed to receive more than quota CAN frames.
> + }
> + /* All packets processed */
> + if (num_pkts < quota) {
> + napi_complete(napi);
> + priv->ier |= RCAR_CAN_IER_RXFIE;
> + writeb(priv->ier, &priv->regs->ier);
> + }
> + return num_pkts;
> +}
> +
> +static int rcar_can_do_set_mode(struct net_device *ndev, enum can_mode mode)
> +{
> + switch (mode) {
> + case CAN_MODE_START:
> + rcar_can_start(ndev);
> + netif_wake_queue(ndev);
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int rcar_can_get_berr_counter(const struct net_device *dev,
> + struct can_berr_counter *bec)
> +{
> + struct rcar_can_priv *priv = netdev_priv(dev);
> +
> + clk_prepare_enable(priv->clk);
clk_prepare_enable can fail
> + bec->txerr = readb(&priv->regs->tecr);
> + bec->rxerr = readb(&priv->regs->recr);
> + clk_disable_unprepare(priv->clk);
> + return 0;
> +}
> +
> +static int rcar_can_probe(struct platform_device *pdev)
> +{
> + struct rcar_can_platform_data *pdata;
> + struct rcar_can_priv *priv;
> + struct net_device *ndev;
> + struct resource *mem;
> + void __iomem *addr;
> + int err = -ENODEV;
> + int irq;
> +
> + pdata = dev_get_platdata(&pdev->dev);
> + if (!pdata) {
> + dev_err(&pdev->dev, "No platform data provided!\n");
> + goto fail;
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + if (!irq) {
> + dev_err(&pdev->dev, "No IRQ resource\n");
> + goto fail;
> + }
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + addr = devm_ioremap_resource(&pdev->dev, mem);
> + if (IS_ERR(addr)) {
> + err = PTR_ERR(addr);
> + goto fail;
> + }
> +
> + ndev = alloc_candev(sizeof(struct rcar_can_priv), RCAR_CAN_N_ECHO_SKB);
> + if (!ndev) {
> + dev_err(&pdev->dev, "alloc_candev failed\n");
> + err = -ENOMEM;
> + goto fail;
> + }
> +
> + priv = netdev_priv(ndev);
> +
> + priv->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(priv->clk)) {
> + err = PTR_ERR(priv->clk);
> + dev_err(&pdev->dev, "cannot get clock: %d\n", err);
> + goto fail_clk;
> + }
> +
> + ndev->netdev_ops = &rcar_can_netdev_ops;
> + ndev->irq = irq;
> + ndev->flags |= IFF_ECHO;
> + priv->ndev = ndev;
> + priv->regs = addr;
> + priv->clock_select = pdata->clock_select;
> + priv->can.clock.freq = clk_get_rate(priv->clk);
> + priv->can.bittiming_const = &rcar_can_bittiming_const;
> + priv->can.do_set_mode = rcar_can_do_set_mode;
> + priv->can.do_get_berr_counter = rcar_can_get_berr_counter;
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_BERR_REPORTING;
> + platform_set_drvdata(pdev, ndev);
> + SET_NETDEV_DEV(ndev, &pdev->dev);
> + spin_lock_init(&priv->skb_lock);
> +
> + netif_napi_add(ndev, &priv->napi, rcar_can_rx_poll,
> + RCAR_CAN_NAPI_WEIGHT);
> + err = register_candev(ndev);
> + if (err) {
> + dev_err(&pdev->dev, "register_candev() failed\n");
> + goto fail_candev;
> + }
> +
> + devm_can_led_init(ndev);
> +
> + dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%u)\n",
> + priv->regs, ndev->irq);
> +
> + return 0;
> +fail_candev:
> + netif_napi_del(&priv->napi);
> +fail_clk:
> + free_candev(ndev);
> +fail:
> + return err;
> +}
> +
> +static int rcar_can_remove(struct platform_device *pdev)
> +{
> + struct net_device *ndev = platform_get_drvdata(pdev);
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> +
> + unregister_candev(ndev);
> + netif_napi_del(&priv->napi);
> + free_candev(ndev);
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int rcar_can_suspend(struct device *dev)
> +{
> + struct net_device *ndev = dev_get_drvdata(dev);
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + u16 ctlr;
> +
> + if (netif_running(ndev)) {
> + netif_stop_queue(ndev);
> + netif_device_detach(ndev);
> + }
> + ctlr = readw(&priv->regs->ctlr);
> + ctlr |= RCAR_CAN_CTLR_CANM_HALT;
> + writew(ctlr, &priv->regs->ctlr);
> + ctlr |= RCAR_CAN_CTLR_SLPM;
> + writew(ctlr, &priv->regs->ctlr);
> + priv->can.state = CAN_STATE_SLEEPING;
> +
> + clk_disable(priv->clk);
> + return 0;
> +}
> +
> +static int rcar_can_resume(struct device *dev)
> +{
> + struct net_device *ndev = dev_get_drvdata(dev);
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + u16 ctlr;
> +
> + clk_enable(priv->clk);
> +
> + ctlr = readw(&priv->regs->ctlr);
> + ctlr &= ~RCAR_CAN_CTLR_SLPM;
> + writew(ctlr, &priv->regs->ctlr);
> + ctlr &= ~RCAR_CAN_CTLR_CANM;
> + writew(ctlr, &priv->regs->ctlr);
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> + if (netif_running(ndev)) {
> + netif_device_attach(ndev);
> + netif_start_queue(ndev);
> + }
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(rcar_can_pm_ops, rcar_can_suspend, rcar_can_resume);
> +
> +static struct platform_driver rcar_can_driver = {
> + .driver = {
> + .name = RCAR_CAN_DRV_NAME,
> + .owner = THIS_MODULE,
> + .pm = &rcar_can_pm_ops,
> + },
> + .probe = rcar_can_probe,
> + .remove = rcar_can_remove,
> +};
> +
> +module_platform_driver(rcar_can_driver);
> +
> +MODULE_AUTHOR("Cogent Embedded, Inc.");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("CAN driver for Renesas R-Car SoC");
> +MODULE_ALIAS("platform:" RCAR_CAN_DRV_NAME);
> Index: linux-can-next/include/linux/can/platform/rcar_can.h
> ===================================================================
> --- /dev/null
> +++ linux-can-next/include/linux/can/platform/rcar_can.h
> @@ -0,0 +1,15 @@
> +#ifndef _CAN_PLATFORM_RCAR_CAN_H_
> +#define _CAN_PLATFORM_RCAR_CAN_H_
> +
> +#include <linux/types.h>
> +
> +/* Clock Select Register settings */
> +#define CLKR_CLKEXT 3 /* Externally input clock */
> +#define CLKR_CLKP2 1 /* Peripheral clock (clkp2) */
> +#define CLKR_CLKP1 0 /* Peripheral clock (clkp1) */
Please make it an enum
> +
> +struct rcar_can_platform_data {
> + u8 clock_select; /* Clock source select */
> +};
> +
> +#endif /* !_CAN_PLATFORM_RCAR_CAN_H_ */
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox