* Re: [PATCH] net: wireless: rtlwifi: rtl8192de: fw.c: Remove unused function
From: Kalle Valo @ 2014-12-24 15:23 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Larry Finger, Chaoming Li, John W. Linville, linux-wireless,
netdev, linux-kernel
In-Reply-To: <1417989613-18300-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se> writes:
> Remove the function rtl92d_set_fw_pwrmode_cmd() that is not used anywhere.
>
> This was partially found by using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Thanks, applied to wireless-drivers-next.git.
But in the future, to make my work easier, please simplify your patch
titles. You do not have to put the full directory structure there. I now
manually changed it to:
commit 18e0c0bf3a5ea0f54384149570274d535341dc06
Author: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Date: Sun Dec 7 23:00:13 2014 +0100
rtlwifi: rtl8192de: fw.c: Remove unused function
Most important is that the title begins with the name of the driver
(rtlwifi, iwlwifi, ath9k and so on) and is not too long. That way it's a
lot easier for me and Johannes to manage the wireless patches in
patchwork.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH next] drivers/net/wireless/ath/wil6210/debugfs.c: Use 'uint64_t' instead of 'cycles_t' to avoid warnings
From: Chen Gang @ 2014-12-24 15:22 UTC (permalink / raw)
To: Kalle Valo
Cc: qca_vkondrat, linux-wireless, wil6210, netdev@vger.kernel.org,
linux-next
In-Reply-To: <8761d19j6z.fsf@kamboji.qca.qualcomm.com>
On 12/24/2014 11:11 PM, Kalle Valo wrote:
> Chen Gang <gang.chen.5i5j@gmail.com> writes:
>
>> Oh, sorry, I forgot the word wrap (after change a new pc). I shall send
>> patch v2 for it.
>
> While at it, could you also please simplify the title. You don't have to
> put the full directory there, it just makes it more difficult to read.
> This should be enough:
>
> wil6210: Use 'uint64_t' instead of 'cycles_t' to avoid warnings
>
Oh, I have sent patch v2, if necessary, I will send patch v3 for it.
(if necessary, please let me know)
> I see quite a few patches with titles like 'drivers: net: wireless: ath:
> fix foo'. Where an earth is that coming from? Bad documentation
> somewhere?
>
I guess, this can let the patch makers have a easier life, e.g. for me,
I may send patches for various sub-systems (arch/*, drivers/*, net/*,
also another projects ...).
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply
* Re: [PATCH next] drivers/net/wireless/ath/wil6210/debugfs.c: Use 'uint64_t' instead of 'cycles_t' to avoid warnings
From: Kalle Valo @ 2014-12-24 15:11 UTC (permalink / raw)
To: Chen Gang
Cc: qca_vkondrat-A+ZNKFmMK5xy9aJCnZT0Uw,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
wil6210-A+ZNKFmMK5xy9aJCnZT0Uw, netdev@vger.kernel.org,
linux-next-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <549AD54F.3020605-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Chen Gang <gang.chen.5i5j-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> Oh, sorry, I forgot the word wrap (after change a new pc). I shall send
> patch v2 for it.
While at it, could you also please simplify the title. You don't have to
put the full directory there, it just makes it more difficult to read.
This should be enough:
wil6210: Use 'uint64_t' instead of 'cycles_t' to avoid warnings
I see quite a few patches with titles like 'drivers: net: wireless: ath:
fix foo'. Where an earth is that coming from? Bad documentation
somewhere?
--
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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
* [PATCH next v2] drivers/net/wireless/ath/wil6210/debugfs.c: Use 'uint64_t' instead of 'cycles_t' to avoid warnings
From: Chen Gang @ 2014-12-24 15:08 UTC (permalink / raw)
To: qca_vkondrat, kvalo; +Cc: linux-wireless, wil6210, netdev, linux-next
do_div() checks the type strictly. 'cycles_t' may be 32-bit under quite
a few architectures (parisc, arm, avr32 ...). So use 'uint64_t' instead
of, the related warning (with allmodconfig under parisc):
CC [M] drivers/net/wireless/ath/wil6210/debugfs.o
In file included from arch/parisc/include/generated/asm/div64.h:1:0,
from include/linux/kernel.h:124,
from include/linux/list.h:8,
from include/linux/module.h:9,
from drivers/net/wireless/ath/wil6210/debugfs.c:17:
drivers/net/wireless/ath/wil6210/debugfs.c: In function ‘wil_vring_debugfs_show’:
include/asm-generic/div64.h:43:28: warning: comparison of distinct pointer types lacks a cast
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
^
drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion of macro ‘do_div’
do_div(idle, total);
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from ./include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/list.h:4,
from include/linux/module.h:9,
from drivers/net/wireless/ath/wil6210/debugfs.c:17:
include/asm-generic/div64.h:44:18: warning: right shift count >= width of type [-Wshift-count-overflow]
if (likely(((n) >> 32) == 0)) { \
^
include/linux/compiler.h:159:40: note: in definition of macro ‘likely’
# define likely(x) __builtin_expect(!!(x), 1)
^
drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion of macro ‘do_div’
do_div(idle, total);
^
In file included from arch/parisc/include/generated/asm/div64.h:1:0,
from include/linux/kernel.h:124,
from include/linux/list.h:8,
from include/linux/module.h:9,
from drivers/net/wireless/ath/wil6210/debugfs.c:17:
include/asm-generic/div64.h:48:22: warning: passing argument 1 of ‘__div64_32’ from incompatible pointer type [-Wincompatible-pointer-types]
__rem = __div64_32(&(n), __base); \
^
drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion of macro ‘do_div’
do_div(idle, total);
^
include/asm-generic/div64.h:35:17: note: expected ‘uint64_t * {aka long long unsigned int *}’ but argument is of type ‘cycles_t * {aka long unsigned int *}’
extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
^
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
drivers/net/wireless/ath/wil6210/debugfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index 4e6e145..cd991fa 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -101,8 +101,8 @@ static int wil_vring_debugfs_show(struct seq_file *s, void *data)
char name[10];
/* performance monitoring */
cycles_t now = get_cycles();
- cycles_t idle = txdata->idle * 100;
- cycles_t total = now - txdata->begin;
+ uint64_t idle = txdata->idle * 100;
+ uint64_t total = now - txdata->begin;
do_div(idle, total);
txdata->begin = now;
--
1.7.9.5
^ permalink raw reply related
* Re: [iproute2] tc: Show classes more hierarchically]
From: Jamal Hadi Salim @ 2014-12-24 15:07 UTC (permalink / raw)
To: Vadim Kochan, Thomas Graf
Cc: Daniel Borkmann, Marcelo Ricardo Leitner, Stephen Hemminger,
netdev@vger.kernel.org, Werner Almesberger, Cong Wang
In-Reply-To: <CAMw6YJK8OBCTNMF2xf3fa_Hnutxf6paEaVkfDKDJSbqUSPchKQ@mail.gmail.com>
On 12/24/14 08:59, Vadim Kochan wrote:
> On Thu, Dec 18, 2014 at 3:58 PM, Thomas Graf <tgraf@suug.ch> wrote:
>> On 12/18/14 at 03:46pm, Vadim Kochan wrote:
>>> On Thu, Dec 18, 2014 at 02:47:38PM +0100, Daniel Borkmann wrote:
>>>> On 12/18/2014 02:16 PM, Vadim Kochan wrote:
>>>> ...
>>>>> The problem that this is huge now but looks better visually, I am
>>>>> thinking also about to possibiliy to show only some part of tree by specified parent id ...
>>>>
>>>> I definitely like this work!
>>>>
>>>> Just thinking out loud, what about an output option for tc which is plain
>>>> DOT [1], thus it can be handed over for rendering in tools like graphviz?
>>>>
>>>> It won't require any dependencies either as it's just plaintext.
>>>>
>>>> [1] https://en.wikipedia.org/wiki/DOT_%28graph_description_language%29
>>>
>>> Yeah, good idea :-)
>>
>> tcng had something like that ;-)
>
> I did not use tcng, but I think that it would be good to adopt some
> useful features to the tc.
>
Werner unfortunately wont be able to make netdev01 ;-> but we could have
this discussion there.
For the record I am enjoying seeing these patches as well;->
cheers,
jamal
^ permalink raw reply
* [PATCH next] net: netfilter: nfnetlink_cthelper: Remove 'const' and '&' to avoid warnings
From: Chen Gang @ 2014-12-24 15:04 UTC (permalink / raw)
To: pablo, kaber, kadlec
Cc: David Miller, netfilter-devel, coreteam, netdev,
linux-kernel@vger.kernel.org
The related code can be simplified, and also can avoid related warnings
(with allmodconfig under parisc):
CC [M] net/netfilter/nfnetlink_cthelper.o
net/netfilter/nfnetlink_cthelper.c: In function ‘nfnl_cthelper_from_nlattr’:
net/netfilter/nfnetlink_cthelper.c:97:9: warning: passing argument 1 o ‘memcpy’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-array-qualifiers]
memcpy(&help->data, nla_data(attr), help->helper->data_len);
^
In file included from include/linux/string.h:17:0,
from include/uapi/linux/uuid.h:25,
from include/linux/uuid.h:23,
from include/linux/mod_devicetable.h:12,
from ./arch/parisc/include/asm/hardware.h:4,
from ./arch/parisc/include/asm/processor.h:15,
from ./arch/parisc/include/asm/spinlock.h:6,
from ./arch/parisc/include/asm/atomic.h:21,
from include/linux/atomic.h:4,
from ./arch/parisc/include/asm/bitops.h:12,
from include/linux/bitops.h:36,
from include/linux/kernel.h:10,
from include/linux/list.h:8,
from include/linux/module.h:9,
from net/netfilter/nfnetlink_cthelper.c:11:
./arch/parisc/include/asm/string.h:8:8: note: expected ‘void *’ but argument is of type ‘const char (*)[]’
void * memcpy(void * dest,const void *src,size_t count);
^
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
net/netfilter/nfnetlink_cthelper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
index 9e287cb..a5599fc 100644
--- a/net/netfilter/nfnetlink_cthelper.c
+++ b/net/netfilter/nfnetlink_cthelper.c
@@ -86,7 +86,7 @@ nfnl_cthelper_parse_tuple(struct nf_conntrack_tuple *tuple,
static int
nfnl_cthelper_from_nlattr(struct nlattr *attr, struct nf_conn *ct)
{
- const struct nf_conn_help *help = nfct_help(ct);
+ struct nf_conn_help *help = nfct_help(ct);
if (attr == NULL)
return -EINVAL;
@@ -94,7 +94,7 @@ nfnl_cthelper_from_nlattr(struct nlattr *attr, struct nf_conn *ct)
if (help->helper->data_len == 0)
return -EINVAL;
- memcpy(&help->data, nla_data(attr), help->helper->data_len);
+ memcpy(help->data, nla_data(attr), help->helper->data_len);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] can: kvaser_usb: Add support for the Usbcan-II family
From: Ahmed S. Darwish @ 2014-12-24 15:04 UTC (permalink / raw)
To: Olivier Sobrie
Cc: Oliver Hartkopp, Wolfgang Grandegger, Marc Kleine-Budde,
David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <CAEM=fMqNnq9QYLiR8WXJ0wN2V=1B1TouxaLB_JLrYOtKvwxBkA@mail.gmail.com>
Hi Olivier,
On Wed, Dec 24, 2014 at 01:36:27PM +0100, Olivier Sobrie wrote:
> Hello Ahmed,
>
> On Tue, Dec 23, 2014 at 05:53:11PM +0200, Ahmed S. Darwish wrote:
> > From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> >
> > CAN to USB interfaces sold by the Swedish manufacturer Kvaser are
> > divided into two major families: 'Leaf', and 'UsbcanII'. From an
> > Operating System perspective, the firmware of both families behave
> > in a not too drastically different fashion.
> >
> > This patch adds support for the UsbcanII family of devices to the
> > current Kvaser Leaf-only driver.
> >
> > CAN frames sending, receiving, and error handling paths has been
> > tested using the dual-channel "Kvaser USBcan II HS/LS" dongle. It
> > should also work nicely with other products in the same category.
> >
>
> Good, thank you :-) I'll try to test the patch during the next
> week-end. Small remarks below.
>
Great! thanks and Merry Christmas :-)
> > Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> > ---
> > drivers/net/can/usb/kvaser_usb.c | 630 +++++++++++++++++++++++++++++++--------
> > 1 file changed, 505 insertions(+), 125 deletions(-)
> >
> > (Generated over 3.19.0-rc1 + generic bugfix at
> > can-kvaser_usb-Don-t-free-packets-when-tight-on-URBs.patch)
> >
> > diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> > index 34c35d8..e7076da 100644
> > --- a/drivers/net/can/usb/kvaser_usb.c
> > +++ b/drivers/net/can/usb/kvaser_usb.c
> > @@ -6,12 +6,15 @@
> > * Parts of this driver are based on the following:
> > * - Kvaser linux leaf driver (version 4.78)
> > * - CAN driver for esd CAN-USB/2
> > + * - Kvaser linux usbcanII driver (version 5.3)
> > *
> > * Copyright (C) 2002-2006 KVASER AB, Sweden. All rights reserved.
> > * Copyright (C) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
> > * Copyright (C) 2012 Olivier Sobrie <olivier@sobrie.be>
> > + * Copyright (C) 2014 Valeo Corporation
> > */
> >
> > +#include <linux/kernel.h>
> > #include <linux/completion.h>
> > #include <linux/module.h>
> > #include <linux/netdevice.h>
> > @@ -21,6 +24,18 @@
> > #include <linux/can/dev.h>
> > #include <linux/can/error.h>
> >
> > +#define MAX(a, b) ((a) > (b) ? (a) : (b))
>
> There is a max(a, b) macro in <linux/kernel.h>.
>
Quite true, but it unfortunately fails when the symbol is
used in array size declaration as in below:
struct kvaser_usb {
...
struct kvaser_usb_net_priv *nets[MAX_NET_DEVICES];
...
}
include/linux/kernel.h:713:19: error: braced-group within
expression allowed only inside a function
#define max(x, y) ({ \
^
> > +
> > +/*
> > + * Kvaser USB CAN dongles are divided into two major families:
> > + * - Leaf: Based on Renesas M32C, running firmware labeled as 'filo'
> > + * - UsbcanII: Based on Renesas M16C, running firmware labeled as 'helios'
> > + */
> > +enum kvaser_usb_family {
> > + KVASER_LEAF,
> > + KVASER_USBCAN,
> > +};
> > +
> > #define MAX_TX_URBS 16
> > #define MAX_RX_URBS 4
> > #define START_TIMEOUT 1000 /* msecs */
> > @@ -29,9 +44,12 @@
> > #define USB_RECV_TIMEOUT 1000 /* msecs */
> > #define RX_BUFFER_SIZE 3072
> > #define CAN_USB_CLOCK 8000000
> > -#define MAX_NET_DEVICES 3
> > +#define LEAF_MAX_NET_DEVICES 3
> > +#define USBCAN_MAX_NET_DEVICES 2
> > +#define MAX_NET_DEVICES MAX(LEAF_MAX_NET_DEVICES, \
> > + USBCAN_MAX_NET_DEVICES)
> >
> > -/* Kvaser USB devices */
> > +/* Leaf USB devices */
> > #define KVASER_VENDOR_ID 0x0bfd
> > #define USB_LEAF_DEVEL_PRODUCT_ID 10
> > #define USB_LEAF_LITE_PRODUCT_ID 11
> > @@ -55,6 +73,16 @@
> > #define USB_CAN_R_PRODUCT_ID 39
> > #define USB_LEAF_LITE_V2_PRODUCT_ID 288
> > #define USB_MINI_PCIE_HS_PRODUCT_ID 289
> > +#define LEAF_PRODUCT_ID(id) (id >= USB_LEAF_DEVEL_PRODUCT_ID && \
> > + id <= USB_MINI_PCIE_HS_PRODUCT_ID)
> > +
> > +/* USBCANII devices */
> > +#define USB_USBCAN_REVB_PRODUCT_ID 2
> > +#define USB_VCI2_PRODUCT_ID 3
> > +#define USB_USBCAN2_PRODUCT_ID 4
> > +#define USB_MEMORATOR_PRODUCT_ID 5
> > +#define USBCAN_PRODUCT_ID(id) (id >= USB_USBCAN_REVB_PRODUCT_ID && \
> > + id <= USB_MEMORATOR_PRODUCT_ID)
> >
> > /* USB devices features */
> > #define KVASER_HAS_SILENT_MODE BIT(0)
> > @@ -73,7 +101,7 @@
> > #define MSG_FLAG_TX_ACK BIT(6)
> > #define MSG_FLAG_TX_REQUEST BIT(7)
> >
> > -/* Can states */
> > +/* Can states (M16C CxSTRH register) */
> > #define M16C_STATE_BUS_RESET BIT(0)
> > #define M16C_STATE_BUS_ERROR BIT(4)
> > #define M16C_STATE_BUS_PASSIVE BIT(5)
> > @@ -98,7 +126,13 @@
> > #define CMD_START_CHIP_REPLY 27
> > #define CMD_STOP_CHIP 28
> > #define CMD_STOP_CHIP_REPLY 29
> > -#define CMD_GET_CARD_INFO2 32
> > +#define CMD_READ_CLOCK 30
> > +#define CMD_READ_CLOCK_REPLY 31
> > +
> > +#define LEAF_CMD_GET_CARD_INFO2 32
> > +#define USBCAN_CMD_RESET_CLOCK 32
> > +#define USBCAN_CMD_CLOCK_OVERFLOW_EVENT 33
> > +
> > #define CMD_GET_CARD_INFO 34
> > #define CMD_GET_CARD_INFO_REPLY 35
> > #define CMD_GET_SOFTWARE_INFO 38
> > @@ -108,8 +142,9 @@
> > #define CMD_RESET_ERROR_COUNTER 49
> > #define CMD_TX_ACKNOWLEDGE 50
> > #define CMD_CAN_ERROR_EVENT 51
> > -#define CMD_USB_THROTTLE 77
> > -#define CMD_LOG_MESSAGE 106
> > +
> > +#define LEAF_CMD_USB_THROTTLE 77
> > +#define LEAF_CMD_LOG_MESSAGE 106
> >
> > /* error factors */
> > #define M16C_EF_ACKE BIT(0)
> > @@ -121,6 +156,13 @@
> > #define M16C_EF_RCVE BIT(6)
> > #define M16C_EF_TRE BIT(7)
> >
> > +/* Only Leaf-based devices can report M16C error factors,
> > + * thus define our own error status flags for USBCAN */
> > +#define USBCAN_ERROR_STATE_NONE 0
> > +#define USBCAN_ERROR_STATE_TX_ERROR BIT(0)
> > +#define USBCAN_ERROR_STATE_RX_ERROR BIT(1)
> > +#define USBCAN_ERROR_STATE_BUSERROR BIT(2)
> > +
> > /* bittiming parameters */
> > #define KVASER_USB_TSEG1_MIN 1
> > #define KVASER_USB_TSEG1_MAX 16
> > @@ -137,7 +179,7 @@
> > #define KVASER_CTRL_MODE_SELFRECEPTION 3
> > #define KVASER_CTRL_MODE_OFF 4
> >
> > -/* log message */
> > +/* Extended CAN identifier flag */
> > #define KVASER_EXTENDED_FRAME BIT(31)
> >
> > struct kvaser_msg_simple {
> > @@ -148,30 +190,55 @@ struct kvaser_msg_simple {
> > struct kvaser_msg_cardinfo {
> > u8 tid;
> > u8 nchannels;
> > - __le32 serial_number;
> > - __le32 padding;
> > + union {
> > + struct {
> > + __le32 serial_number;
> > + __le32 padding;
> > + } __packed leaf0;
> > + struct {
> > + __le32 serial_number_low;
> > + __le32 serial_number_high;
> > + } __packed usbcan0;
> > + } __packed;
> > __le32 clock_resolution;
> > __le32 mfgdate;
> > u8 ean[8];
> > u8 hw_revision;
> > - u8 usb_hs_mode;
> > - __le16 padding2;
> > + union {
> > + struct {
> > + u8 usb_hs_mode;
> > + } __packed leaf1;
> > + struct {
> > + u8 padding;
> > + } __packed usbcan1;
> > + } __packed;
> > + __le16 padding;
> > } __packed;
> >
> > struct kvaser_msg_cardinfo2 {
> > u8 tid;
> > - u8 channel;
> > + u8 reserved;
> > u8 pcb_id[24];
> > __le32 oem_unlock_code;
> > } __packed;
> >
> > -struct kvaser_msg_softinfo {
> > +struct leaf_msg_softinfo {
> > u8 tid;
> > - u8 channel;
> > + u8 padding0;
> > __le32 sw_options;
> > __le32 fw_version;
> > __le16 max_outstanding_tx;
> > - __le16 padding[9];
> > + __le16 padding1[9];
> > +} __packed;
> > +
> > +struct usbcan_msg_softinfo {
> > + u8 tid;
> > + u8 fw_name[5];
> > + __le16 max_outstanding_tx;
> > + u8 padding[6];
> > + __le32 fw_version;
> > + __le16 checksum;
> > + __le16 sw_options;
> > } __packed;
> >
> > struct kvaser_msg_busparams {
> > @@ -188,36 +255,86 @@ struct kvaser_msg_tx_can {
> > u8 channel;
> > u8 tid;
> > u8 msg[14];
> > - u8 padding;
> > - u8 flags;
> > + union {
> > + struct {
> > + u8 padding;
> > + u8 flags;
> > + } __packed leaf;
> > + struct {
> > + u8 flags;
> > + u8 padding;
> > + } __packed usbcan;
> > + } __packed;
> > +} __packed;
> > +
> > +struct kvaser_msg_rx_can_header {
> > + u8 channel;
> > + u8 flag;
> > } __packed;
> >
> > -struct kvaser_msg_rx_can {
> > +struct leaf_msg_rx_can {
> > u8 channel;
> > u8 flag;
> > +
> > __le16 time[3];
> > u8 msg[14];
> > } __packed;
> >
> > -struct kvaser_msg_chip_state_event {
> > +struct usbcan_msg_rx_can {
> > + u8 channel;
> > + u8 flag;
> > +
> > + u8 msg[14];
> > + __le16 time;
> > +} __packed;
> > +
> > +struct leaf_msg_chip_state_event {
> > u8 tid;
> > u8 channel;
> > +
> > __le16 time[3];
> > u8 tx_errors_count;
> > u8 rx_errors_count;
> > +
> > u8 status;
> > u8 padding[3];
> > } __packed;
> >
> > -struct kvaser_msg_tx_acknowledge {
> > +struct usbcan_msg_chip_state_event {
> > + u8 tid;
> > + u8 channel;
> > +
> > + u8 tx_errors_count;
> > + u8 rx_errors_count;
> > + __le16 time;
> > +
> > + u8 status;
> > + u8 padding[3];
> > +} __packed;
> > +
> > +struct kvaser_msg_tx_acknowledge_header {
> > + u8 channel;
> > + u8 tid;
> > +};
> > +
> > +struct leaf_msg_tx_acknowledge {
> > u8 channel;
> > u8 tid;
> > +
> > __le16 time[3];
> > u8 flags;
> > u8 time_offset;
> > } __packed;
> >
> > -struct kvaser_msg_error_event {
> > +struct usbcan_msg_tx_acknowledge {
> > + u8 channel;
> > + u8 tid;
> > +
> > + __le16 time;
> > + __le16 padding;
> > +} __packed;
> > +
> > +struct leaf_msg_error_event {
> > u8 tid;
> > u8 flags;
> > __le16 time[3];
> > @@ -229,6 +346,18 @@ struct kvaser_msg_error_event {
> > u8 error_factor;
> > } __packed;
> >
> > +struct usbcan_msg_error_event {
> > + u8 tid;
> > + u8 padding;
> > + u8 tx_errors_count_ch0;
> > + u8 rx_errors_count_ch0;
> > + u8 tx_errors_count_ch1;
> > + u8 rx_errors_count_ch1;
> > + u8 status_ch0;
> > + u8 status_ch1;
> > + __le16 time;
> > +} __packed;
> > +
> > struct kvaser_msg_ctrl_mode {
> > u8 tid;
> > u8 channel;
> > @@ -243,7 +372,7 @@ struct kvaser_msg_flush_queue {
> > u8 padding[3];
> > } __packed;
> >
> > -struct kvaser_msg_log_message {
> > +struct leaf_msg_log_message {
> > u8 channel;
> > u8 flags;
> > __le16 time[3];
> > @@ -260,19 +389,49 @@ struct kvaser_msg {
> > struct kvaser_msg_simple simple;
> > struct kvaser_msg_cardinfo cardinfo;
> > struct kvaser_msg_cardinfo2 cardinfo2;
> > - struct kvaser_msg_softinfo softinfo;
> > struct kvaser_msg_busparams busparams;
> > +
> > + struct kvaser_msg_rx_can_header rx_can_header;
> > + struct kvaser_msg_tx_acknowledge_header tx_acknowledge_header;
> > +
> > + union {
> > + struct leaf_msg_softinfo softinfo;
> > + struct leaf_msg_rx_can rx_can;
> > + struct leaf_msg_chip_state_event chip_state_event;
> > + struct leaf_msg_tx_acknowledge tx_acknowledge;
> > + struct leaf_msg_error_event error_event;
> > + struct leaf_msg_log_message log_message;
> > + } __packed leaf;
> > +
> > + union {
> > + struct usbcan_msg_softinfo softinfo;
> > + struct usbcan_msg_rx_can rx_can;
> > + struct usbcan_msg_chip_state_event chip_state_event;
> > + struct usbcan_msg_tx_acknowledge tx_acknowledge;
> > + struct usbcan_msg_error_event error_event;
> > + } __packed usbcan;
> > +
> > struct kvaser_msg_tx_can tx_can;
> > - struct kvaser_msg_rx_can rx_can;
> > - struct kvaser_msg_chip_state_event chip_state_event;
> > - struct kvaser_msg_tx_acknowledge tx_acknowledge;
> > - struct kvaser_msg_error_event error_event;
> > struct kvaser_msg_ctrl_mode ctrl_mode;
> > struct kvaser_msg_flush_queue flush_queue;
> > - struct kvaser_msg_log_message log_message;
> > } u;
> > } __packed;
> >
> > +/* Leaf/USBCAN-agnostic summary of an error event.
> > + * No M16C error factors for USBCAN-based devices. */
> > +struct kvaser_error_summary {
> > + u8 channel, status, txerr, rxerr;
> > + union {
> > + struct {
> > + u8 error_factor;
> > + } leaf;
> > + struct {
> > + u8 other_ch_status;
> > + u8 error_state;
> > + } usbcan;
> > + };
> > +};
> > +
> > struct kvaser_usb_tx_urb_context {
> > struct kvaser_usb_net_priv *priv;
> > u32 echo_index;
> > @@ -288,6 +447,8 @@ struct kvaser_usb {
> >
> > u32 fw_version;
> > unsigned int nchannels;
> > + enum kvaser_usb_family family;
> > + unsigned int max_channels;
> >
> > bool rxinitdone;
> > void *rxbuf[MAX_RX_URBS];
> > @@ -311,6 +472,7 @@ struct kvaser_usb_net_priv {
> > };
> >
> > static const struct usb_device_id kvaser_usb_table[] = {
> > + /* Leaf family IDs */
> > { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
> > { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
> > { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
> > @@ -360,6 +522,17 @@ static const struct usb_device_id kvaser_usb_table[] = {
> > .driver_info = KVASER_HAS_TXRX_ERRORS },
> > { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_V2_PRODUCT_ID) },
> > { USB_DEVICE(KVASER_VENDOR_ID, USB_MINI_PCIE_HS_PRODUCT_ID) },
> > +
> > + /* USBCANII family IDs */
> > + { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN2_PRODUCT_ID),
> > + .driver_info = KVASER_HAS_TXRX_ERRORS },
> > + { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN_REVB_PRODUCT_ID),
> > + .driver_info = KVASER_HAS_TXRX_ERRORS },
> > + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMORATOR_PRODUCT_ID),
> > + .driver_info = KVASER_HAS_TXRX_ERRORS },
> > + { USB_DEVICE(KVASER_VENDOR_ID, USB_VCI2_PRODUCT_ID),
> > + .driver_info = KVASER_HAS_TXRX_ERRORS },
> > +
> > { }
> > };
> > MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
> > @@ -463,7 +636,18 @@ static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
> > if (err)
> > return err;
> >
> > - dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
> > + switch (dev->family) {
> > + case KVASER_LEAF:
> > + dev->fw_version = le32_to_cpu(msg.u.leaf.softinfo.fw_version);
> > + break;
> > + case KVASER_USBCAN:
> > + dev->fw_version = le32_to_cpu(msg.u.usbcan.softinfo.fw_version);
> > + break;
> > + default:
> > + dev_err(dev->udev->dev.parent,
> > + "Invalid device family (%d)\n", dev->family);
> > + return -EINVAL;
> > + }
> >
> > return 0;
> > }
> > @@ -482,7 +666,7 @@ static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
> > return err;
> >
> > dev->nchannels = msg.u.cardinfo.nchannels;
> > - if (dev->nchannels > MAX_NET_DEVICES)
> > + if (dev->nchannels > dev->max_channels)
> > return -EINVAL;
>
> IMHO, you can keep MAX_NET_DEVICES here.
>
The UsbcanII firmware hardcodes a maximum of 2 channels in
its protocol. This is unfortunately due to its inability to
tell whether an error event is from CAN channel 0 or ch 1,
and also due to its error_event format:
struct usbcan_msg_error_event {
u8 tid;
u8 padding;
u8 tx_errors_count_ch0;
u8 rx_errors_count_ch0;
u8 tx_errors_count_ch1;
u8 rx_errors_count_ch1;
u8 status_ch0;
u8 status_ch1;
__le16 time;
} __packed;
But since we have MAX_NET_DEVICES = 3, and given the above,
if the UsbcanII firmware reported to us having more than 2
channels, then it's:
a) most probably a memory corruption bug either in the firmware
or in the driver
b) an updated device/firmware we cannot support yet, since
we cannot arbitrate the origin of error events quite correctly
(especially in the case of CAN_ERR_BUSERROR, where the error
counters stays the same and we have to resort to other hacks.
Kindly check usbcan_report_error_if_applicable().)
So allowing more than 2 channels given the current set of
affairs will really induce correctness problems :-(
> >
> > return 0;
> > @@ -496,8 +680,10 @@ static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
> > struct kvaser_usb_net_priv *priv;
> > struct sk_buff *skb;
> > struct can_frame *cf;
> > - u8 channel = msg->u.tx_acknowledge.channel;
> > - u8 tid = msg->u.tx_acknowledge.tid;
> > + u8 channel, tid;
> > +
> > + channel = msg->u.tx_acknowledge_header.channel;
> > + tid = msg->u.tx_acknowledge_header.tid;
> >
> > if (channel >= dev->nchannels) {
> > dev_err(dev->udev->dev.parent,
> > @@ -615,37 +801,83 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> > priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> > }
> >
> > -static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> > - const struct kvaser_msg *msg)
> > +static void kvaser_report_error_event(const struct kvaser_usb *dev,
> > + struct kvaser_error_summary *es);
> > +
> > +/*
> > + * Report error to userspace iff the controller's errors counter has
> > + * increased, or we're the only channel seeing the bus error state.
> > + *
> > + * As reported by USBCAN sheets, "the CAN controller has difficulties
> > + * to tell whether an error frame arrived on channel 1 or on channel 2."
> > + * Thus, error counters are compared with their earlier values to
> > + * determine which channel was responsible for the error event.
> > + */
> > +static void usbcan_report_error_if_applicable(const struct kvaser_usb *dev,
> > + struct kvaser_error_summary *es)
> > {
> > - struct can_frame *cf;
> > - struct sk_buff *skb;
> > - struct net_device_stats *stats;
> > struct kvaser_usb_net_priv *priv;
> > - unsigned int new_state;
> > - u8 channel, status, txerr, rxerr, error_factor;
> > + int old_tx_err_count, old_rx_err_count, channel, report_error;
> > +
> > + channel = es->channel;
> > + if (channel >= dev->nchannels) {
> > + dev_err(dev->udev->dev.parent,
> > + "Invalid channel number (%d)\n", channel);
> > + return;
> > + }
> > +
> > + priv = dev->nets[channel];
> > + old_tx_err_count = priv->bec.txerr;
> > + old_rx_err_count = priv->bec.rxerr;
> > +
> > + report_error = 0;
> > + if (es->txerr > old_tx_err_count) {
> > + es->usbcan.error_state |= USBCAN_ERROR_STATE_TX_ERROR;
> > + report_error = 1;
> > + }
> > + if (es->rxerr > old_rx_err_count) {
> > + es->usbcan.error_state |= USBCAN_ERROR_STATE_RX_ERROR;
> > + report_error = 1;
> > + }
> > + if ((es->status & M16C_STATE_BUS_ERROR) &&
> > + !(es->usbcan.other_ch_status & M16C_STATE_BUS_ERROR)) {
> > + es->usbcan.error_state |= USBCAN_ERROR_STATE_BUSERROR;
> > + report_error = 1;
> > + }
> > +
> > + if (report_error)
> > + kvaser_report_error_event(dev, es);
> > +}
> > +
> > +/*
> > + * Extract error summary from a Leaf-based device error message
> > + */
> > +static void leaf_extract_error_from_msg(const struct kvaser_usb *dev,
> > + const struct kvaser_msg *msg)
> > +{
> > + struct kvaser_error_summary es = { 0, };
> >
> > switch (msg->id) {
> > case CMD_CAN_ERROR_EVENT:
> > - channel = msg->u.error_event.channel;
> > - status = msg->u.error_event.status;
> > - txerr = msg->u.error_event.tx_errors_count;
> > - rxerr = msg->u.error_event.rx_errors_count;
> > - error_factor = msg->u.error_event.error_factor;
> > + es.channel = msg->u.leaf.error_event.channel;
> > + es.status = msg->u.leaf.error_event.status;
> > + es.txerr = msg->u.leaf.error_event.tx_errors_count;
> > + es.rxerr = msg->u.leaf.error_event.rx_errors_count;
> > + es.leaf.error_factor = msg->u.leaf.error_event.error_factor;
> > break;
> > - case CMD_LOG_MESSAGE:
> > - channel = msg->u.log_message.channel;
> > - status = msg->u.log_message.data[0];
> > - txerr = msg->u.log_message.data[2];
> > - rxerr = msg->u.log_message.data[3];
> > - error_factor = msg->u.log_message.data[1];
> > + case LEAF_CMD_LOG_MESSAGE:
> > + es.channel = msg->u.leaf.log_message.channel;
> > + es.status = msg->u.leaf.log_message.data[0];
> > + es.txerr = msg->u.leaf.log_message.data[2];
> > + es.rxerr = msg->u.leaf.log_message.data[3];
> > + es.leaf.error_factor = msg->u.leaf.log_message.data[1];
> > break;
> > case CMD_CHIP_STATE_EVENT:
> > - channel = msg->u.chip_state_event.channel;
> > - status = msg->u.chip_state_event.status;
> > - txerr = msg->u.chip_state_event.tx_errors_count;
> > - rxerr = msg->u.chip_state_event.rx_errors_count;
> > - error_factor = 0;
> > + es.channel = msg->u.leaf.chip_state_event.channel;
> > + es.status = msg->u.leaf.chip_state_event.status;
> > + es.txerr = msg->u.leaf.chip_state_event.tx_errors_count;
> > + es.rxerr = msg->u.leaf.chip_state_event.rx_errors_count;
> > + es.leaf.error_factor = 0;
> > break;
> > default:
> > dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> > @@ -653,16 +885,92 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> > return;
> > }
> >
> > - if (channel >= dev->nchannels) {
> > + kvaser_report_error_event(dev, &es);
> > +}
> > +
> > +/*
> > + * Extract summary from a USBCANII-based device error message.
> > + */
> > +static void usbcan_extract_error_from_msg(const struct kvaser_usb *dev,
> > + const struct kvaser_msg *msg)
> > +{
> > + struct kvaser_error_summary es = { 0, };
> > +
> > + switch (msg->id) {
> > +
> > + /* Sometimes errors are sent as unsolicited chip state events */
> > + case CMD_CHIP_STATE_EVENT:
> > + es.channel = msg->u.usbcan.chip_state_event.channel;
> > + es.status = msg->u.usbcan.chip_state_event.status;
> > + es.txerr = msg->u.usbcan.chip_state_event.tx_errors_count;
> > + es.rxerr = msg->u.usbcan.chip_state_event.rx_errors_count;
> > + usbcan_report_error_if_applicable(dev, &es);
> > + break;
> > +
> > + case CMD_CAN_ERROR_EVENT:
> > + es.channel = 0;
> > + es.status = msg->u.usbcan.error_event.status_ch0;
> > + es.txerr = msg->u.usbcan.error_event.tx_errors_count_ch0;
> > + es.rxerr = msg->u.usbcan.error_event.rx_errors_count_ch0;
> > + es.usbcan.other_ch_status =
> > + msg->u.usbcan.error_event.status_ch1;
> > + usbcan_report_error_if_applicable(dev, &es);
> > +
> > + /* For error events, the USBCAN firmware does not support
> > + * more than 2 channels: ch0, and ch1. */
> > + if (dev->nchannels > 1) {
> > + es.channel = 1;
> > + es.status = msg->u.usbcan.error_event.status_ch1;
> > + es.txerr = msg->u.usbcan.error_event.tx_errors_count_ch1;
> > + es.rxerr = msg->u.usbcan.error_event.rx_errors_count_ch1;
> > + es.usbcan.other_ch_status =
> > + msg->u.usbcan.error_event.status_ch0;
> > + usbcan_report_error_if_applicable(dev, &es);
> > + }
> > + break;
> > +
> > + default:
> > + dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> > + msg->id);
> > + }
> > +}
> > +
> > +static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> > + const struct kvaser_msg *msg)
> > +{
> > + switch (dev->family) {
> > + case KVASER_LEAF:
> > + leaf_extract_error_from_msg(dev, msg);
> > + break;
> > + case KVASER_USBCAN:
> > + usbcan_extract_error_from_msg(dev, msg);
> > + break;
> > + default:
> > dev_err(dev->udev->dev.parent,
> > - "Invalid channel number (%d)\n", channel);
> > + "Invalid device family (%d)\n", dev->family);
> > return;
> > }
> > +}
> >
> > - priv = dev->nets[channel];
> > +static void kvaser_report_error_event(const struct kvaser_usb *dev,
> > + struct kvaser_error_summary *es)
> > +{
> > + struct can_frame *cf;
> > + struct sk_buff *skb;
> > + struct net_device_stats *stats;
> > + struct kvaser_usb_net_priv *priv;
> > + unsigned int new_state;
> > +
> > + if (es->channel >= dev->nchannels) {
> > + dev_err(dev->udev->dev.parent,
> > + "Invalid channel number (%d)\n", es->channel);
> > + return;
> > + }
> > +
> > + priv = dev->nets[es->channel];
> > stats = &priv->netdev->stats;
> >
> > - if (status & M16C_STATE_BUS_RESET) {
> > + if (es->status & M16C_STATE_BUS_RESET) {
> > kvaser_usb_unlink_tx_urbs(priv);
> > return;
> > }
> > @@ -675,9 +983,9 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> >
> > new_state = priv->can.state;
> >
> > - netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
> > + netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
> >
> > - if (status & M16C_STATE_BUS_OFF) {
> > + if (es->status & M16C_STATE_BUS_OFF) {
> > cf->can_id |= CAN_ERR_BUSOFF;
> >
> > priv->can.can_stats.bus_off++;
> > @@ -687,12 +995,12 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> > netif_carrier_off(priv->netdev);
> >
> > new_state = CAN_STATE_BUS_OFF;
> > - } else if (status & M16C_STATE_BUS_PASSIVE) {
> > + } else if (es->status & M16C_STATE_BUS_PASSIVE) {
> > if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
> > cf->can_id |= CAN_ERR_CRTL;
> >
> > - if (txerr || rxerr)
> > - cf->data[1] = (txerr > rxerr)
> > + if (es->txerr || es->rxerr)
> > + cf->data[1] = (es->txerr > es->rxerr)
> > ? CAN_ERR_CRTL_TX_PASSIVE
> > : CAN_ERR_CRTL_RX_PASSIVE;
> > else
> > @@ -703,13 +1011,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> > }
> >
> > new_state = CAN_STATE_ERROR_PASSIVE;
> > - }
> > -
> > - if (status == M16C_STATE_BUS_ERROR) {
> > + } else if (es->status & M16C_STATE_BUS_ERROR) {
> > if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> > - ((txerr >= 96) || (rxerr >= 96))) {
> > + ((es->txerr >= 96) || (es->rxerr >= 96))) {
> > cf->can_id |= CAN_ERR_CRTL;
> > - cf->data[1] = (txerr > rxerr)
> > + cf->data[1] = (es->txerr > es->rxerr)
> > ? CAN_ERR_CRTL_TX_WARNING
> > : CAN_ERR_CRTL_RX_WARNING;
> >
> > @@ -723,7 +1029,7 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> > }
> > }
> >
> > - if (!status) {
> > + if (!es->status) {
> > cf->can_id |= CAN_ERR_PROT;
> > cf->data[2] = CAN_ERR_PROT_ACTIVE;
> >
> > @@ -739,34 +1045,52 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> > priv->can.can_stats.restarts++;
> > }
> >
> > - if (error_factor) {
> > - priv->can.can_stats.bus_error++;
> > - stats->rx_errors++;
> > -
> > - cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> > -
> > - if (error_factor & M16C_EF_ACKE)
> > - cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
> > - if (error_factor & M16C_EF_CRCE)
> > - cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> > - CAN_ERR_PROT_LOC_CRC_DEL);
> > - if (error_factor & M16C_EF_FORME)
> > - cf->data[2] |= CAN_ERR_PROT_FORM;
> > - if (error_factor & M16C_EF_STFE)
> > - cf->data[2] |= CAN_ERR_PROT_STUFF;
> > - if (error_factor & M16C_EF_BITE0)
> > - cf->data[2] |= CAN_ERR_PROT_BIT0;
> > - if (error_factor & M16C_EF_BITE1)
> > - cf->data[2] |= CAN_ERR_PROT_BIT1;
> > - if (error_factor & M16C_EF_TRE)
> > - cf->data[2] |= CAN_ERR_PROT_TX;
> > + switch (dev->family) {
> > + case KVASER_LEAF:
> > + if (es->leaf.error_factor) {
> > + priv->can.can_stats.bus_error++;
> > + stats->rx_errors++;
> > +
> > + cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> > +
> > + if (es->leaf.error_factor & M16C_EF_ACKE)
> > + cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
> > + if (es->leaf.error_factor & M16C_EF_CRCE)
> > + cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> > + CAN_ERR_PROT_LOC_CRC_DEL);
> > + if (es->leaf.error_factor & M16C_EF_FORME)
> > + cf->data[2] |= CAN_ERR_PROT_FORM;
> > + if (es->leaf.error_factor & M16C_EF_STFE)
> > + cf->data[2] |= CAN_ERR_PROT_STUFF;
> > + if (es->leaf.error_factor & M16C_EF_BITE0)
> > + cf->data[2] |= CAN_ERR_PROT_BIT0;
> > + if (es->leaf.error_factor & M16C_EF_BITE1)
> > + cf->data[2] |= CAN_ERR_PROT_BIT1;
> > + if (es->leaf.error_factor & M16C_EF_TRE)
> > + cf->data[2] |= CAN_ERR_PROT_TX;
> > + }
> > + break;
> > + case KVASER_USBCAN:
> > + if (es->usbcan.error_state & USBCAN_ERROR_STATE_TX_ERROR)
> > + stats->tx_errors++;
> > + if (es->usbcan.error_state & USBCAN_ERROR_STATE_RX_ERROR)
> > + stats->rx_errors++;
> > + if (es->usbcan.error_state & USBCAN_ERROR_STATE_BUSERROR) {
> > + priv->can.can_stats.bus_error++;
> > + cf->can_id |= CAN_ERR_BUSERROR;
> > + }
> > + break;
> > + default:
> > + dev_err(dev->udev->dev.parent,
> > + "Invalid device family (%d)\n", dev->family);
> > + goto err;
> > }
> >
> > - cf->data[6] = txerr;
> > - cf->data[7] = rxerr;
> > + cf->data[6] = es->txerr;
> > + cf->data[7] = es->rxerr;
> >
> > - priv->bec.txerr = txerr;
> > - priv->bec.rxerr = rxerr;
> > + priv->bec.txerr = es->txerr;
> > + priv->bec.rxerr = es->rxerr;
> >
> > priv->can.state = new_state;
> >
> > @@ -774,6 +1098,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> >
> > stats->rx_packets++;
> > stats->rx_bytes += cf->can_dlc;
> > +
> > + return;
> > +
> > +err:
> > + dev_kfree_skb(skb);
> > }
> >
> > static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> > @@ -783,16 +1112,16 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> > struct sk_buff *skb;
> > struct net_device_stats *stats = &priv->netdev->stats;
> >
> > - if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> > + if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
> > MSG_FLAG_NERR)) {
> > netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n",
> > - msg->u.rx_can.flag);
> > + msg->u.rx_can_header.flag);
> >
> > stats->rx_errors++;
> > return;
> > }
> >
> > - if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
> > + if (msg->u.rx_can_header.flag & MSG_FLAG_OVERRUN) {
> > skb = alloc_can_err_skb(priv->netdev, &cf);
> > if (!skb) {
> > stats->rx_dropped++;
> > @@ -819,7 +1148,8 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> > struct can_frame *cf;
> > struct sk_buff *skb;
> > struct net_device_stats *stats;
> > - u8 channel = msg->u.rx_can.channel;
> > + u8 channel = msg->u.rx_can_header.channel;
> > + const u8 *rx_msg;
> >
> > if (channel >= dev->nchannels) {
> > dev_err(dev->udev->dev.parent,
> > @@ -830,19 +1160,32 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> > priv = dev->nets[channel];
> > stats = &priv->netdev->stats;
> >
> > - if ((msg->u.rx_can.flag & MSG_FLAG_ERROR_FRAME) &&
> > - (msg->id == CMD_LOG_MESSAGE)) {
> > + if ((msg->u.rx_can_header.flag & MSG_FLAG_ERROR_FRAME) &&
> > + (dev->family == KVASER_LEAF && msg->id == LEAF_CMD_LOG_MESSAGE)) {
> > kvaser_usb_rx_error(dev, msg);
> > return;
> > - } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> > - MSG_FLAG_NERR |
> > - MSG_FLAG_OVERRUN)) {
> > + } else if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
> > + MSG_FLAG_NERR |
> > + MSG_FLAG_OVERRUN)) {
> > kvaser_usb_rx_can_err(priv, msg);
> > return;
> > - } else if (msg->u.rx_can.flag & ~MSG_FLAG_REMOTE_FRAME) {
> > + } else if (msg->u.rx_can_header.flag & ~MSG_FLAG_REMOTE_FRAME) {
> > netdev_warn(priv->netdev,
> > "Unhandled frame (flags: 0x%02x)",
> > - msg->u.rx_can.flag);
> > + msg->u.rx_can_header.flag);
> > + return;
> > + }
> > +
> > + switch (dev->family) {
> > + case KVASER_LEAF:
> > + rx_msg = msg->u.leaf.rx_can.msg;
> > + break;
> > + case KVASER_USBCAN:
> > + rx_msg = msg->u.usbcan.rx_can.msg;
> > + break;
> > + default:
> > + dev_err(dev->udev->dev.parent,
> > + "Invalid device family (%d)\n", dev->family);
> > return;
> > }
> >
> > @@ -852,38 +1195,37 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> > return;
> > }
> >
> > - if (msg->id == CMD_LOG_MESSAGE) {
> > - cf->can_id = le32_to_cpu(msg->u.log_message.id);
> > + if (dev->family == KVASER_LEAF && msg->id == LEAF_CMD_LOG_MESSAGE) {
> > + cf->can_id = le32_to_cpu(msg->u.leaf.log_message.id);
> > if (cf->can_id & KVASER_EXTENDED_FRAME)
> > cf->can_id &= CAN_EFF_MASK | CAN_EFF_FLAG;
> > else
> > cf->can_id &= CAN_SFF_MASK;
> >
> > - cf->can_dlc = get_can_dlc(msg->u.log_message.dlc);
> > + cf->can_dlc = get_can_dlc(msg->u.leaf.log_message.dlc);
> >
> > - if (msg->u.log_message.flags & MSG_FLAG_REMOTE_FRAME)
> > + if (msg->u.leaf.log_message.flags & MSG_FLAG_REMOTE_FRAME)
> > cf->can_id |= CAN_RTR_FLAG;
> > else
> > - memcpy(cf->data, &msg->u.log_message.data,
> > + memcpy(cf->data, &msg->u.leaf.log_message.data,
> > cf->can_dlc);
> > } else {
> > - cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
> > - (msg->u.rx_can.msg[1] & 0x3f);
> > + cf->can_id = ((rx_msg[0] & 0x1f) << 6) | (rx_msg[1] & 0x3f);
> >
> > if (msg->id == CMD_RX_EXT_MESSAGE) {
> > cf->can_id <<= 18;
> > - cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
> > - ((msg->u.rx_can.msg[3] & 0xff) << 6) |
> > - (msg->u.rx_can.msg[4] & 0x3f);
> > + cf->can_id |= ((rx_msg[2] & 0x0f) << 14) |
> > + ((rx_msg[3] & 0xff) << 6) |
> > + (rx_msg[4] & 0x3f);
> > cf->can_id |= CAN_EFF_FLAG;
> > }
> >
> > - cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
> > + cf->can_dlc = get_can_dlc(rx_msg[5]);
> >
> > - if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME)
> > + if (msg->u.rx_can_header.flag & MSG_FLAG_REMOTE_FRAME)
> > cf->can_id |= CAN_RTR_FLAG;
> > else
> > - memcpy(cf->data, &msg->u.rx_can.msg[6],
> > + memcpy(cf->data, &rx_msg[6],
> > cf->can_dlc);
> > }
> >
> > @@ -947,7 +1289,12 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
> >
> > case CMD_RX_STD_MESSAGE:
> > case CMD_RX_EXT_MESSAGE:
> > - case CMD_LOG_MESSAGE:
> > + kvaser_usb_rx_can_msg(dev, msg);
> > + break;
> > +
> > + case LEAF_CMD_LOG_MESSAGE:
> > + if (dev->family != KVASER_LEAF)
> > + goto warn;
> > kvaser_usb_rx_can_msg(dev, msg);
> > break;
> >
> > @@ -960,8 +1307,14 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
> > kvaser_usb_tx_acknowledge(dev, msg);
> > break;
> >
> > + /* Ignored messages */
> > + case USBCAN_CMD_CLOCK_OVERFLOW_EVENT:
> > + if (dev->family != KVASER_USBCAN)
> > + goto warn;
> > + break;
> > +
> > default:
> > - dev_warn(dev->udev->dev.parent,
> > +warn: dev_warn(dev->udev->dev.parent,
> > "Unhandled message (%d)\n", msg->id);
> > break;
> > }
> > @@ -1181,7 +1534,7 @@ static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
> > dev->rxbuf[i],
> > dev->rxbuf_dma[i]);
> >
> > - for (i = 0; i < MAX_NET_DEVICES; i++) {
> > + for (i = 0; i < dev->max_channels; i++) {
>
> here too... or replace it by nchannels.
>
Yes, indeed. nchannels is the correct choice here, especially since
kvaser_usb_init_one() is called "dev->nchannels" times too.
> > struct kvaser_usb_net_priv *priv = dev->nets[i];
> >
> > if (priv)
> > @@ -1286,6 +1639,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> > struct kvaser_msg *msg;
> > int i, err;
> > int ret = NETDEV_TX_OK;
> > + uint8_t *msg_tx_can_flags;
> > bool kfree_skb_on_error = true;
> >
> > if (can_dropped_invalid_skb(netdev, skb))
> > @@ -1306,9 +1660,23 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> >
> > msg = buf;
> > msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
> > - msg->u.tx_can.flags = 0;
> > msg->u.tx_can.channel = priv->channel;
> >
> > + switch (dev->family) {
> > + case KVASER_LEAF:
> > + msg_tx_can_flags = &msg->u.tx_can.leaf.flags;
> > + break;
> > + case KVASER_USBCAN:
> > + msg_tx_can_flags = &msg->u.tx_can.usbcan.flags;
> > + break;
> > + default:
> > + dev_err(dev->udev->dev.parent,
> > + "Invalid device family (%d)\n", dev->family);
> > + goto releasebuf;
> > + }
> > +
> > + *msg_tx_can_flags = 0;
> > +
> > if (cf->can_id & CAN_EFF_FLAG) {
> > msg->id = CMD_TX_EXT_MESSAGE;
> > msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
> > @@ -1326,7 +1694,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> > memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc);
> >
> > if (cf->can_id & CAN_RTR_FLAG)
> > - msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME;
> > + *msg_tx_can_flags |= MSG_FLAG_REMOTE_FRAME;
> >
> > for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) {
> > if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
> > @@ -1596,6 +1964,18 @@ static int kvaser_usb_probe(struct usb_interface *intf,
> > if (!dev)
> > return -ENOMEM;
> >
> > + if (LEAF_PRODUCT_ID(id->idProduct)) {
> > + dev->family = KVASER_LEAF;
> > + dev->max_channels = LEAF_MAX_NET_DEVICES;
> > + } else if (USBCAN_PRODUCT_ID(id->idProduct)) {
> > + dev->family = KVASER_USBCAN;
> > + dev->max_channels = USBCAN_MAX_NET_DEVICES;
> > + } else {
> > + dev_err(&intf->dev, "Product ID (%d) does not belong to any "
> > + "known Kvaser USB family", id->idProduct);
> > + return -ENODEV;
> > + }
> > +
>
> Is it really required to keep max_channels in the kvaser_usb structure?
> If I looked correctly, you use this variable as a replacement for
> MAX_NET_DEVICES in the code and MAX_NET_DEVICES is only used in probe
> and disconnect functions. I think it can even be replaced by nchannels
> in the disconnect path. So I also think that it don't need to be in the
> kvaser_usb structure.
>
hmmm.. given the current state of error arbitration explained
above, where I cannot accept a dev->nchannels > 2, I guess we
have two options:
a) Remove max_channels, and hardcode the channels count
correctness logic as follows:
dev->nchannels = msg.u.cardinfo.nchannels;
if ((dev->family == USBCAN && dev->nchannels > USBCAN_MAX_NET_DEVICES)
|| (dev->family == LEAF && dev->nchannels > LEAF_MAX_NET_DEVICES))
return -EINVAL
b) Leave max_channels in 'struct kvaser_usb' as is.
I personally prefer the solution at 'b)' but I can do it as
in 'a)' if you prefer :-)
> > err = kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
> > if (err) {
> > dev_err(&intf->dev, "Cannot get usb endpoint(s)");
> > @@ -1608,7 +1988,7 @@ static int kvaser_usb_probe(struct usb_interface *intf,
> >
> > usb_set_intfdata(intf, dev);
> >
> > - for (i = 0; i < MAX_NET_DEVICES; i++)
> > + for (i = 0; i < dev->max_channels; i++)
> > kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i);
>
> Someone reported me that recent leaf firmwares go in trouble when
> you send a command for a channel that does not exist. Instead of
> max_channels, you can use nchannels here and move the reset command
> in the kvaser_usb_init_one() function.
> I've a patch for this but It is not tested yet. I'll send it next week-end after
> I did some tests.
>
Great. I guess I can submit a 3-patch series now
(kfree_skb fix + the above fix + driver).
> >
> > err = kvaser_usb_get_software_info(dev);
>
> Thank you,
>
Thanks a lot for your review.
P.S. the Gmail mailer you've used messed badly with the patch
code identation; I had to manually restore it back to make the
discussion meaningful for others :-)
Regards,
--
Darwish
^ permalink raw reply
* Re: [PATCH next] drivers/net/wireless/ath/wil6210/debugfs.c: Use 'uint64_t' instead of 'cycles_t' to avoid warnings
From: Chen Gang @ 2014-12-24 15:01 UTC (permalink / raw)
To: qca_vkondrat, kvalo
Cc: linux-wireless, wil6210, netdev@vger.kernel.org, linux-next
In-Reply-To: <549AD067.80608@gmail.com>
Oh, sorry, I forgot the word wrap (after change a new pc). I shall send
patch v2 for it.
On 12/24/2014 10:40 PM, Chen Gang wrote:
> do_div() checks the type strictly. 'cycles_t' may be 32-bit under quite
> a few architectures (parisc, arm, avr32 ...). So use 'uint64_t' instead
> of, the related warning (with allmodconfig under parisc):
>
> CC [M] drivers/net/wireless/ath/wil6210/debugfs.o
> In file included from arch/parisc/include/generated/asm/div64.h:1:0,
> from include/linux/kernel.h:124,
> from include/linux/list.h:8,
> from include/linux/module.h:9,
> from drivers/net/wireless/ath/wil6210/debugfs.c:17:
> drivers/net/wireless/ath/wil6210/debugfs.c: In function
> ‘wil_vring_debugfs_show’:
> include/asm-generic/div64.h:43:28: warning: comparison of distinct
> pointer types lacks a cast
> (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
> ^
> drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion
> of macro ‘do_div’
> do_div(idle, total);
> ^
> In file included from include/uapi/linux/stddef.h:1:0,
> from include/linux/stddef.h:4,
> from ./include/uapi/linux/posix_types.h:4,
> from include/uapi/linux/types.h:13,
> from include/linux/types.h:5,
> from include/linux/list.h:4,
> from include/linux/module.h:9,
> from drivers/net/wireless/ath/wil6210/debugfs.c:17:
> include/asm-generic/div64.h:44:18: warning: right shift count >= width
> of type [-Wshift-count-overflow]
> if (likely(((n) >> 32) == 0)) { \
> ^
> include/linux/compiler.h:159:40: note: in definition of macro ‘likely’
> # define likely(x) __builtin_expect(!!(x), 1)
> ^
> drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion
> of macro ‘do_div’
> do_div(idle, total);
> ^
> In file included from arch/parisc/include/generated/asm/div64.h:1:0,
> from include/linux/kernel.h:124,
> from include/linux/list.h:8,
> from include/linux/module.h:9,
> from drivers/net/wireless/ath/wil6210/debugfs.c:17:
> include/asm-generic/div64.h:48:22: warning: passing argument 1 of
> ‘__div64_32’ from incompatible pointer type [-Wincompatible-pointer-types]
> __rem = __div64_32(&(n), __base); \
> ^
> drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion
> of macro ‘do_div’
> do_div(idle, total);
> ^
> include/asm-generic/div64.h:35:17: note: expected ‘uint64_t * {aka
> long long unsigned int *}’ but argument is of type ‘cycles_t * {aka long
> unsigned int *}’
> extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
> ^
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> drivers/net/wireless/ath/wil6210/debugfs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c
> b/drivers/net/wireless/ath/wil6210/debugfs.c
> index 4e6e145..cd991fa 100644
> --- a/drivers/net/wireless/ath/wil6210/debugfs.c
> +++ b/drivers/net/wireless/ath/wil6210/debugfs.c
> @@ -101,8 +101,8 @@ static int wil_vring_debugfs_show(struct seq_file
> *s, void *data)
> char name[10];
> /* performance monitoring */
> cycles_t now = get_cycles();
> - cycles_t idle = txdata->idle * 100;
> - cycles_t total = now - txdata->begin;
> + uint64_t idle = txdata->idle * 100;
> + uint64_t total = now - txdata->begin;
>
> do_div(idle, total);
> txdata->begin = now;
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply
* [PATCH next] drivers/net/wireless/ath/wil6210/debugfs.c: Use 'uint64_t' instead of 'cycles_t' to avoid warnings
From: Chen Gang @ 2014-12-24 14:40 UTC (permalink / raw)
To: qca_vkondrat, kvalo
Cc: linux-wireless, wil6210, netdev@vger.kernel.org, linux-next
do_div() checks the type strictly. 'cycles_t' may be 32-bit under quite
a few architectures (parisc, arm, avr32 ...). So use 'uint64_t' instead
of, the related warning (with allmodconfig under parisc):
CC [M] drivers/net/wireless/ath/wil6210/debugfs.o
In file included from arch/parisc/include/generated/asm/div64.h:1:0,
from include/linux/kernel.h:124,
from include/linux/list.h:8,
from include/linux/module.h:9,
from drivers/net/wireless/ath/wil6210/debugfs.c:17:
drivers/net/wireless/ath/wil6210/debugfs.c: In function
‘wil_vring_debugfs_show’:
include/asm-generic/div64.h:43:28: warning: comparison of distinct
pointer types lacks a cast
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
^
drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion
of macro ‘do_div’
do_div(idle, total);
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from ./include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/list.h:4,
from include/linux/module.h:9,
from drivers/net/wireless/ath/wil6210/debugfs.c:17:
include/asm-generic/div64.h:44:18: warning: right shift count >= width
of type [-Wshift-count-overflow]
if (likely(((n) >> 32) == 0)) { \
^
include/linux/compiler.h:159:40: note: in definition of macro ‘likely’
# define likely(x) __builtin_expect(!!(x), 1)
^
drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion
of macro ‘do_div’
do_div(idle, total);
^
In file included from arch/parisc/include/generated/asm/div64.h:1:0,
from include/linux/kernel.h:124,
from include/linux/list.h:8,
from include/linux/module.h:9,
from drivers/net/wireless/ath/wil6210/debugfs.c:17:
include/asm-generic/div64.h:48:22: warning: passing argument 1 of
‘__div64_32’ from incompatible pointer type [-Wincompatible-pointer-types]
__rem = __div64_32(&(n), __base); \
^
drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion
of macro ‘do_div’
do_div(idle, total);
^
include/asm-generic/div64.h:35:17: note: expected ‘uint64_t * {aka
long long unsigned int *}’ but argument is of type ‘cycles_t * {aka long
unsigned int *}’
extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
^
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
drivers/net/wireless/ath/wil6210/debugfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c
b/drivers/net/wireless/ath/wil6210/debugfs.c
index 4e6e145..cd991fa 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -101,8 +101,8 @@ static int wil_vring_debugfs_show(struct seq_file
*s, void *data)
char name[10];
/* performance monitoring */
cycles_t now = get_cycles();
- cycles_t idle = txdata->idle * 100;
- cycles_t total = now - txdata->begin;
+ uint64_t idle = txdata->idle * 100;
+ uint64_t total = now - txdata->begin;
do_div(idle, total);
txdata->begin = now;
--
1.7.9.5
^ permalink raw reply related
* [PATCH next] drivers: net: wireless: ath: wil6210: debugfs: Use 'uint64_t' instead of 'cycles_t' to avoid warnings
From: Chen Gang @ 2014-12-24 14:29 UTC (permalink / raw)
To: qca_vkondrat, kvalo
Cc: linux-wireless, wil6210, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
do_div() checks the type strictly. 'cycles_t' may be 32-bit under quite
a few architectures (parisc, arm, avr32 ...). So use 'uint64_t' instead
of, the related warning (with allmodconfig undr parisc):
CC [M] drivers/net/wireless/ath/wil6210/debugfs.o
In file included from arch/parisc/include/generated/asm/div64.h:1:0,
from include/linux/kernel.h:124,
from include/linux/list.h:8,
from include/linux/module.h:9,
from drivers/net/wireless/ath/wil6210/debugfs.c:17:
drivers/net/wireless/ath/wil6210/debugfs.c: In function
‘wil_vring_debugfs_show’:
include/asm-generic/div64.h:43:28: warning: comparison of distinct
pointer types lacks a cast
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
^
drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion
of macro ‘do_div’
do_div(idle, total);
^
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from ./include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/list.h:4,
from include/linux/module.h:9,
from drivers/net/wireless/ath/wil6210/debugfs.c:17:
include/asm-generic/div64.h:44:18: warning: right shift count >= width
of type [-Wshift-count-overflow]
if (likely(((n) >> 32) == 0)) { \
^
include/linux/compiler.h:159:40: note: in definition of macro ‘likely’
# define likely(x) __builtin_expect(!!(x), 1)
^
drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion
of macro ‘do_div’
do_div(idle, total);
^
In file included from arch/parisc/include/generated/asm/div64.h:1:0,
from include/linux/kernel.h:124,
from include/linux/list.h:8,
from include/linux/module.h:9,
from drivers/net/wireless/ath/wil6210/debugfs.c:17:
include/asm-generic/div64.h:48:22: warning: passing argument 1 of
‘__div64_32’ from incompatible pointer type [-Wincompatible-pointer-types]
__rem = __div64_32(&(n), __base); \
^
drivers/net/wireless/ath/wil6210/debugfs.c:107:4: note: in expansion
of macro ‘do_div’
do_div(idle, total);
^
include/asm-generic/div64.h:35:17: note: expected ‘uint64_t * {aka
long long unsigned int *}’ but argument is of type ‘cycles_t * {aka long
unsigned int *}’
extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
^
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
drivers/net/wireless/ath/wil6210/debugfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c
b/drivers/net/wireless/ath/wil6210/debugfs.c
index 4e6e145..cd991fa 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -101,8 +101,8 @@ static int wil_vring_debugfs_show(struct seq_file
*s, void *data)
char name[10];
/* performance monitoring */
cycles_t now = get_cycles();
- cycles_t idle = txdata->idle * 100;
- cycles_t total = now - txdata->begin;
+ uint64_t idle = txdata->idle * 100;
+ uint64_t total = now - txdata->begin;
do_div(idle, total);
txdata->begin = now;
--
1.7.9.5
^ permalink raw reply related
* Re: [iproute2] tc: Show classes more hierarchically]
From: Vadim Kochan @ 2014-12-24 13:59 UTC (permalink / raw)
To: Thomas Graf
Cc: Daniel Borkmann, Marcelo Ricardo Leitner, Stephen Hemminger,
netdev@vger.kernel.org, Jamal Hadi Salim
In-Reply-To: <20141218135848.GB16239@casper.infradead.org>
On Thu, Dec 18, 2014 at 3:58 PM, Thomas Graf <tgraf@suug.ch> wrote:
> On 12/18/14 at 03:46pm, Vadim Kochan wrote:
>> On Thu, Dec 18, 2014 at 02:47:38PM +0100, Daniel Borkmann wrote:
>> > On 12/18/2014 02:16 PM, Vadim Kochan wrote:
>> > ...
>> > >The problem that this is huge now but looks better visually, I am
>> > >thinking also about to possibiliy to show only some part of tree by specified parent id ...
>> >
>> > I definitely like this work!
>> >
>> > Just thinking out loud, what about an output option for tc which is plain
>> > DOT [1], thus it can be handed over for rendering in tools like graphviz?
>> >
>> > It won't require any dependencies either as it's just plaintext.
>> >
>> > [1] https://en.wikipedia.org/wiki/DOT_%28graph_description_language%29
>>
>> Yeah, good idea :-)
>
> tcng had something like that ;-)
I did not use tcng, but I think that it would be good to adopt some
useful features to the tc.
Regards,
^ permalink raw reply
* Re: ipv6: oops in datagram.c line 260
From: Chris Ruehl @ 2014-12-24 13:42 UTC (permalink / raw)
To: netdev; +Cc: davem, steffen.klassert
In-Reply-To: <5487DD65.60800@gtsys.com.hk>
On Wednesday, December 10, 2014 01:43 PM, Chris Ruehl wrote:
> Hi all,
>
> We running a Dell server which crash frequently with (dell crash video snapshot)
> vanilla 3.14.25
>
> Capture viewed here: http://www.gtsys.com.hk/~chris/datagram_c_line260.png
>
> The capture sadly don't show the full trace, so we lack on information.
> 1st line I can see in the crash video from the idrac : tcp_transmit_skb+0x461
>
> RIP [<ffffffff815da587>] ipv6_local_error+0x17/0x140
>
> The null pointer happen:
> Type "apropos word" to search for commands related to "word"...
> Reading symbols from net/ipv6/datagram.o...done.
> (gdb) list *(ipv6_local_error+0x17)
> 0xae7 is in ipv6_local_error (net/ipv6/datagram.c:260).
> 255 struct ipv6_pinfo *np = inet6_sk(sk);
> 256 struct sock_exterr_skb *serr;
> 257 struct ipv6hdr *iph;
> 258 struct sk_buff *skb;
> 259
> 260 if (!np->recverr)
> 261 return;
> 262
> 263 skb = alloc_skb(sizeof(struct ipv6hdr), GFP_ATOMIC);
> 264 if (!skb)
> (gdb) quit
>
>
> We running a 6in4 with ipsec tunnel on the 6. I found a pull request from
> Steffen Klassert
> here:
> http://article.gmane.org/gmane.linux.network/281469
>
> Which might be relevant to this problem.
>
> For time being I add a
>
> if (np == NULL){
> LIMIT_NETDEBUG(KERN_DEBUG "ipv6_pinfo is NULL\n");
> return;
> }
>
> as work around to stop the server crashing
>
>
> With kind regards
> Chris
>
Catch it!
Update the kernel to 3.14.27 and add a WARN_ON() to the function and catch the
OOPS after 5 Days.
As mentioned we running a IPv6 in IPv4 with a couple of IPSec tunnels on the v6.
Code change:
void ipv6_local_error(struct sock *sk, int err, struct flowi6 *fl6, u32 info)
{
struct ipv6_pinfo *np = inet6_sk(sk);
struct sock_exterr_skb *serr;
struct ipv6hdr *iph;
struct sk_buff *skb;
if (np == NULL){
LIMIT_NETDEBUG(KERN_CRIT "ipv6_pinfo is NULL\n");
WARN_ON(1);
return;
}
[447604.244357] ipv6_pinfo is NULL
[447604.273733] ------------[ cut here ]------------
[447604.303628] WARNING: CPU: 7 PID: 0 at net/ipv6/datagram.c:262
ipv6_local_error+0x16b/0x1a0()
[447604.366173] Modules linked in: ipmi_si vhost_net vhost macvtap macvlan
xt_policy authenc esp6 xfrm4_mode_tunnel xfrm6_mode_tunnel mpt3sas mpt2sas
raid_class scsi_transport_sas mptctl mptbase ipt_MASQUERADE iptable_nat
nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack
ipt_REJECT xt_CHECKSUM iptable_mangle xt_tcpudp ipmi_devintf dell_rbu
ip6table_filter ip6_tables iptable_filter ip_tables ebtable_nat ebtables
x_tables xfrm_user xfrm4_tunnel ipcomp xfrm_ipcomp esp4 ah4 deflate ctr
twofish_generic twofish_avx_x86_64 twofish_x86_64_3way twofish_x86_64
twofish_common camellia_generic camellia_aesni_avx_x86_64 camellia_x86_64
serpent_avx_x86_64 serpent_sse2_x86_64 xts serpent_generic blowfish_generic
blowfish_x86_64 blowfish_common cast5_avx_x86_64 cast5_generic cast_common
des_generic cmac xcbc rmd160 crypto_null af_key xfrm_algo sit ip_tunnel tunnel4
bridge stp llc xfs libcrc32c intel_rapl x86_pkg_temp_thermal intel_powerclamp
coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul gpio_ich
ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul joydev glue_helper
ablk_helper cryptd dcdbas shpchp wmi mei_me mei acpi_power_meter lpc_ich dummy
lp parport hid_generic tg3 usbhid hid ahci megaraid_sas ptp libahci pps_core
[last unloaded: ipmi_si]
[447605.087999] CPU: 7 PID: 0 Comm: swapper/7 Not tainted 3.14.27 #11
[447605.139687] Hardware name: Dell Inc. PowerEdge R420/0CN7CM, BIOS 2.3.3
07/10/2014
[447605.242931] 0000000000000009 ffff8806172e3b48 ffffffff815ffd58 0000000000000000
[447605.349130] ffff8806172e3b80 ffffffff81043c23 ffff8800a16322e8 ffff880037daa1c0
[447605.459659] ffff88000b026800 0000000000000000 ffff880037daa4b8 ffff8806172e3b90
[447605.576385] Call Trace:
[447605.634243] <IRQ> [<ffffffff815ffd58>] dump_stack+0x45/0x56
[447605.692870] [<ffffffff81043c23>] warn_slowpath_common+0x73/0x90
[447605.751097] [<ffffffff81043cf5>] warn_slowpath_null+0x15/0x20
[447605.808000] [<ffffffff815da6db>] ipv6_local_error+0x16b/0x1a0
[447605.863821] [<ffffffff815e29d0>] xfrm6_local_error+0x60/0x90
[447605.918493] [<ffffffff8150b485>] ? skb_dequeue+0x15/0x70
[447605.971871] [<ffffffff815a6cc1>] xfrm_local_error+0x51/0x70
[447606.024218] [<ffffffff8159ca15>] xfrm4_extract_output+0x75/0xb0
[447606.075630] [<ffffffff815a6c5a>] xfrm_inner_extract_output+0x6a/0x80
[447606.126055] [<ffffffff815e27a2>] xfrm6_prepare_output+0x12/0x60
[447606.175310] [<ffffffff815a6ed0>] xfrm_output_resume+0x1f0/0x370
[447606.223406] [<ffffffff8151a486>] ? skb_checksum_help+0x76/0x190
[447606.270572] [<ffffffff815a709b>] xfrm_output+0x3b/0xf0
[447606.316454] [<ffffffff815e2ae0>] ? xfrm6_extract_output+0xe0/0xe0
[447606.361803] [<ffffffff815e2af7>] xfrm6_output_finish+0x17/0x20
[447606.406053] [<ffffffff8159cad6>] xfrm4_output+0x46/0x80
[447606.448694] [<ffffffff81550a80>] ip_local_out+0x20/0x30
[447606.489952] [<ffffffff81550dd5>] ip_queue_xmit+0x135/0x3c0
[447606.530017] [<ffffffff815672e1>] tcp_transmit_skb+0x461/0x8c0
[447606.569362] [<ffffffff8156786e>] tcp_write_xmit+0x12e/0xb20
[447606.607876] [<ffffffff815669ff>] ? tcp_current_mss+0x4f/0x70
[447606.645723] [<ffffffff8156b320>] ? tcp_write_timer_handler+0x1b0/0x1b0
[447606.682837] [<ffffffff81569487>] tcp_send_loss_probe+0x37/0x1f0
[447606.719000] [<ffffffff8156b320>] ? tcp_write_timer_handler+0x1b0/0x1b0
[447606.754537] [<ffffffff8156b1bb>] tcp_write_timer_handler+0x4b/0x1b0
[447606.789266] [<ffffffff8156b320>] ? tcp_write_timer_handler+0x1b0/0x1b0
[447606.823242] [<ffffffff8156b378>] tcp_write_timer+0x58/0x60
[447606.856047] [<ffffffff8104e848>] call_timer_fn.isra.32+0x18/0x80
[447606.888029] [<ffffffff8104ea1a>] run_timer_softirq+0x16a/0x200
[447606.920224] [<ffffffff81047efc>] __do_softirq+0xec/0x250
[447606.951850] [<ffffffff810482f5>] irq_exit+0xf5/0x100
[447606.982665] [<ffffffff8102bc6f>] smp_apic_timer_interrupt+0x3f/0x50
[447607.014382] [<ffffffff8160d98a>] apic_timer_interrupt+0x6a/0x70
[447607.046175] <EOI> [<ffffffff8104f336>] ? get_next_timer_interrupt+0x1d6/0x250
[447607.111311] [<ffffffff814d45a7>] ? cpuidle_enter_state+0x47/0xc0
[447607.145850] [<ffffffff814d45a3>] ? cpuidle_enter_state+0x43/0xc0
[447607.179625] [<ffffffff814d46b6>] cpuidle_idle_call+0x96/0x130
[447607.213531] [<ffffffff8100b909>] arch_cpu_idle+0x9/0x20
[447607.247052] [<ffffffff810925ba>] cpu_startup_entry+0xda/0x1d0
[447607.280775] [<ffffffff81029d22>] start_secondary+0x212/0x2c0
[447607.314555] ---[ end trace 6ff3826b6e4fdf67 ]---
Can someone have a closer look into this problem?
Regards
Chris
^ permalink raw reply
* Re: [PATCH net-next v2] packet: make packet_snd fail on len smaller than l2 header
From: Willem de Bruijn @ 2014-12-24 13:28 UTC (permalink / raw)
To: Jouni Malinen
Cc: Network Development, David Miller, Eric Dumazet, Daniel Borkmann
In-Reply-To: <CANe27jJfsuXd29UXouTgU8dGqb9h8mXuj9N1_Ua6Q_UbZCeCXQ@mail.gmail.com>
On Tue, Dec 23, 2014 at 11:37 AM, Jouni Malinen <jkmalinen@gmail.com> wrote:
> On Wed, Nov 19, 2014 at 8:10 PM, Willem de Bruijn <willemb@google.com> wrote:
>> When sending packets out with PF_PACKET, SOCK_RAW, ensure that the
>> packet is at least as long as the device's expected link layer header.
>> This check already exists in tpacket_snd, but not in packet_snd.
>
> Was this supposed to be refusing zero-length payload following the
> header like the implementation does or accept zero-length payload like
> this commit message seems to imply? Based on the commit message, I'd
> assume 14 byte buffer on Ethernet netdev should have been accepted.
Fair point. This patch just extended the existing check in tpacket_rcv to
cover packet_rcv, so the code is the authoritative answer. But my
commit message is inconsistent with the patch.
> I just noticed that once pulling this commit in into my automated test
> setup, one of the test cases started failing because of the change
> here. That test case was trying to transmit a minimum length Ethernet
> header using raw packet socket. Not that I care too much since I can
> easily change the test case to use one octet longer data to send and
> this was not really a real protocol case, but I wanted to confirm what
> was the expected behavior here since this commit seems to have number
> of inconsistent statements between the commit message, actual
> validation step, debug print, and code comment..
>
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> @@ -2095,6 +2095,18 @@ static void tpacket_destruct_skb(struct sk_buff *skb)
>> +static bool ll_header_truncated(const struct net_device *dev, int len)
>> +{
>> + /* net device doesn't like empty head */
>
> I'm not sure how to interpret "empty head". Is that saying that the
> data following the header should not be empty? Or that header should
> be at least hard_header_len?
I read it as the first interpretation. I do not have an immediate example
of stack or driver code that cannot handle Ethernet frames without
payload, but also do not know of any legitimate use for sending such
frames. Minimum frame length on the wire is even longer. The current
test errs on the side of caution based on that existing use, then.
>
>> + if (unlikely(len <= dev->hard_header_len)) {
>
> That would be rejecting exactly hard_header_len, i.e,., I would have
> expected < instead of <= here based on the commit message.
>
>> + net_warn_ratelimited("%s: packet size is too short (%d < %d)\n",
>> + current->comm, len, dev->hard_header_len);
>
> But that debug print uses < instead of <= which is not consistent with
> the actual condition (and yes, I realize this was there even before
> this commit).
Thanks, good point. When moving that code, I did not read it closely
enough to notice the inconsistency. This looks sloppy; I can send a one
line patch, unless you want to do it yourself.
> - Jouni
^ permalink raw reply
* Re: [PATCH RFC] ipw2200: select CFG80211_WEXT
From: Kalle Valo @ 2014-12-24 13:18 UTC (permalink / raw)
To: Paul Bolle
Cc: Johannes Berg, Stanislav Yakovlev, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1419271817.2317.12.camel@tiscali.nl>
Paul Bolle <pebolle@tiscali.nl> writes:
> Commit 24a0aa212ee2 ("cfg80211: make WEXT compatibility unselectable")
> made it impossible to depend on CFG80211_WEXT. It does still allow to
> select that symbol. (Yes, the commit summary is confusing.)
>
> So make IPW2200 select CFG80211_WEXT, so that the ipw2200 driver can be
> built again.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
Thanks, applied to wireless-drivers.git.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] can: kvaser_usb: Add support for the Usbcan-II family
From: Olivier Sobrie @ 2014-12-24 12:36 UTC (permalink / raw)
To: Ahmed S. Darwish
Cc: Oliver Hartkopp, Wolfgang Grandegger, Marc Kleine-Budde,
David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20141223155311.GA5997@vivalin-002>
Hello Ahmed,
On Tue, Dec 23, 2014 at 05:53:11PM +0200, Ahmed S. Darwish wrote:
> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>
> CAN to USB interfaces sold by the Swedish manufacturer Kvaser are
> divided into two major families: 'Leaf', and 'UsbcanII'. From an
> Operating System perspective, the firmware of both families behave
> in a not too drastically different fashion.
>
> This patch adds support for the UsbcanII family of devices to the
> current Kvaser Leaf-only driver.
>
> CAN frames sending, receiving, and error handling paths has been
> tested using the dual-channel "Kvaser USBcan II HS/LS" dongle. It
> should also work nicely with other products in the same category.
Good, thank you :-) I'll try to test the patch during the next
week-end. Small remarks below.
>
> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> ---
> drivers/net/can/usb/kvaser_usb.c | 630 +++++++++++++++++++++++++++++++--------
> 1 file changed, 505 insertions(+), 125 deletions(-)
>
> (Generated over 3.19.0-rc1 + generic bugfix at
> can-kvaser_usb-Don-t-free-packets-when-tight-on-URBs.patch)
>
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> index 34c35d8..e7076da 100644
> --- a/drivers/net/can/usb/kvaser_usb.c
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -6,12 +6,15 @@
> * Parts of this driver are based on the following:
> * - Kvaser linux leaf driver (version 4.78)
> * - CAN driver for esd CAN-USB/2
> + * - Kvaser linux usbcanII driver (version 5.3)
> *
> * Copyright (C) 2002-2006 KVASER AB, Sweden. All rights reserved.
> * Copyright (C) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
> * Copyright (C) 2012 Olivier Sobrie <olivier@sobrie.be>
> + * Copyright (C) 2014 Valeo Corporation
> */
>
> +#include <linux/kernel.h>
> #include <linux/completion.h>
> #include <linux/module.h>
> #include <linux/netdevice.h>
> @@ -21,6 +24,18 @@
> #include <linux/can/dev.h>
> #include <linux/can/error.h>
>
> +#define MAX(a, b) ((a) > (b) ? (a) : (b))
There is a max(a, b) macro in <linux/kernel.h>.
> +
> +/*
> + * Kvaser USB CAN dongles are divided into two major families:
> + * - Leaf: Based on Renesas M32C, running firmware labeled as 'filo'
> + * - UsbcanII: Based on Renesas M16C, running firmware labeled as 'helios'
> + */
> +enum kvaser_usb_family {
> + KVASER_LEAF,
> + KVASER_USBCAN,
> +};
> +
> #define MAX_TX_URBS 16
> #define MAX_RX_URBS 4
> #define START_TIMEOUT 1000 /* msecs */
> @@ -29,9 +44,12 @@
> #define USB_RECV_TIMEOUT 1000 /* msecs */
> #define RX_BUFFER_SIZE 3072
> #define CAN_USB_CLOCK 8000000
> -#define MAX_NET_DEVICES 3
> +#define LEAF_MAX_NET_DEVICES 3
> +#define USBCAN_MAX_NET_DEVICES 2
> +#define MAX_NET_DEVICES MAX(LEAF_MAX_NET_DEVICES, \
> + USBCAN_MAX_NET_DEVICES)
>
> -/* Kvaser USB devices */
> +/* Leaf USB devices */
> #define KVASER_VENDOR_ID 0x0bfd
> #define USB_LEAF_DEVEL_PRODUCT_ID 10
> #define USB_LEAF_LITE_PRODUCT_ID 11
> @@ -55,6 +73,16 @@
> #define USB_CAN_R_PRODUCT_ID 39
> #define USB_LEAF_LITE_V2_PRODUCT_ID 288
> #define USB_MINI_PCIE_HS_PRODUCT_ID 289
> +#define LEAF_PRODUCT_ID(id) (id >= USB_LEAF_DEVEL_PRODUCT_ID && \
> + id <= USB_MINI_PCIE_HS_PRODUCT_ID)
> +
> +/* USBCANII devices */
> +#define USB_USBCAN_REVB_PRODUCT_ID 2
> +#define USB_VCI2_PRODUCT_ID 3
> +#define USB_USBCAN2_PRODUCT_ID 4
> +#define USB_MEMORATOR_PRODUCT_ID 5
> +#define USBCAN_PRODUCT_ID(id) (id >= USB_USBCAN_REVB_PRODUCT_ID && \
> + id <= USB_MEMORATOR_PRODUCT_ID)
>
> /* USB devices features */
> #define KVASER_HAS_SILENT_MODE BIT(0)
> @@ -73,7 +101,7 @@
> #define MSG_FLAG_TX_ACK BIT(6)
> #define MSG_FLAG_TX_REQUEST BIT(7)
>
> -/* Can states */
> +/* Can states (M16C CxSTRH register) */
> #define M16C_STATE_BUS_RESET BIT(0)
> #define M16C_STATE_BUS_ERROR BIT(4)
> #define M16C_STATE_BUS_PASSIVE BIT(5)
> @@ -98,7 +126,13 @@
> #define CMD_START_CHIP_REPLY 27
> #define CMD_STOP_CHIP 28
> #define CMD_STOP_CHIP_REPLY 29
> -#define CMD_GET_CARD_INFO2 32
> +#define CMD_READ_CLOCK 30
> +#define CMD_READ_CLOCK_REPLY 31
> +
> +#define LEAF_CMD_GET_CARD_INFO2 32
> +#define USBCAN_CMD_RESET_CLOCK 32
> +#define USBCAN_CMD_CLOCK_OVERFLOW_EVENT 33
> +
> #define CMD_GET_CARD_INFO 34
> #define CMD_GET_CARD_INFO_REPLY 35
> #define CMD_GET_SOFTWARE_INFO 38
> @@ -108,8 +142,9 @@
> #define CMD_RESET_ERROR_COUNTER 49
> #define CMD_TX_ACKNOWLEDGE 50
> #define CMD_CAN_ERROR_EVENT 51
> -#define CMD_USB_THROTTLE 77
> -#define CMD_LOG_MESSAGE 106
> +
> +#define LEAF_CMD_USB_THROTTLE 77
> +#define LEAF_CMD_LOG_MESSAGE 106
>
> /* error factors */
> #define M16C_EF_ACKE BIT(0)
> @@ -121,6 +156,13 @@
> #define M16C_EF_RCVE BIT(6)
> #define M16C_EF_TRE BIT(7)
>
> +/* Only Leaf-based devices can report M16C error factors,
> + * thus define our own error status flags for USBCAN */
> +#define USBCAN_ERROR_STATE_NONE 0
> +#define USBCAN_ERROR_STATE_TX_ERROR BIT(0)
> +#define USBCAN_ERROR_STATE_RX_ERROR BIT(1)
> +#define USBCAN_ERROR_STATE_BUSERROR BIT(2)
> +
> /* bittiming parameters */
> #define KVASER_USB_TSEG1_MIN 1
> #define KVASER_USB_TSEG1_MAX 16
> @@ -137,7 +179,7 @@
> #define KVASER_CTRL_MODE_SELFRECEPTION 3
> #define KVASER_CTRL_MODE_OFF 4
>
> -/* log message */
> +/* Extended CAN identifier flag */
> #define KVASER_EXTENDED_FRAME BIT(31)
>
> struct kvaser_msg_simple {
> @@ -148,30 +190,55 @@ struct kvaser_msg_simple {
> struct kvaser_msg_cardinfo {
> u8 tid;
> u8 nchannels;
> - __le32 serial_number;
> - __le32 padding;
> + union {
> + struct {
> + __le32 serial_number;
> + __le32 padding;
> + } __packed leaf0;
> + struct {
> + __le32 serial_number_low;
> + __le32 serial_number_high;
> + } __packed usbcan0;
> + } __packed;
> __le32 clock_resolution;
> __le32 mfgdate;
> u8 ean[8];
> u8 hw_revision;
> - u8 usb_hs_mode;
> - __le16 padding2;
> + union {
> + struct {
> + u8 usb_hs_mode;
> + } __packed leaf1;
> + struct {
> + u8 padding;
> + } __packed usbcan1;
> + } __packed;
> + __le16 padding;
> } __packed;
>
> struct kvaser_msg_cardinfo2 {
> u8 tid;
> - u8 channel;
> + u8 reserved;
> u8 pcb_id[24];
> __le32 oem_unlock_code;
> } __packed;
>
> -struct kvaser_msg_softinfo {
> +struct leaf_msg_softinfo {
> u8 tid;
> - u8 channel;
> + u8 padding0;
> __le32 sw_options;
> __le32 fw_version;
> __le16 max_outstanding_tx;
> - __le16 padding[9];
> + __le16 padding1[9];
> +} __packed;
> +
> +struct usbcan_msg_softinfo {
> + u8 tid;
> + u8 fw_name[5];
> + __le16 max_outstanding_tx;
> + u8 padding[6];
> + __le32 fw_version;
> + __le16 checksum;
> + __le16 sw_options;
> } __packed;
>
> struct kvaser_msg_busparams {
> @@ -188,36 +255,86 @@ struct kvaser_msg_tx_can {
> u8 channel;
> u8 tid;
> u8 msg[14];
> - u8 padding;
> - u8 flags;
> + union {
> + struct {
> + u8 padding;
> + u8 flags;
> + } __packed leaf;
> + struct {
> + u8 flags;
> + u8 padding;
> + } __packed usbcan;
> + } __packed;
> +} __packed;
> +
> +struct kvaser_msg_rx_can_header {
> + u8 channel;
> + u8 flag;
> } __packed;
>
> -struct kvaser_msg_rx_can {
> +struct leaf_msg_rx_can {
> u8 channel;
> u8 flag;
> +
> __le16 time[3];
> u8 msg[14];
> } __packed;
>
> -struct kvaser_msg_chip_state_event {
> +struct usbcan_msg_rx_can {
> + u8 channel;
> + u8 flag;
> +
> + u8 msg[14];
> + __le16 time;
> +} __packed;
> +
> +struct leaf_msg_chip_state_event {
> u8 tid;
> u8 channel;
> +
> __le16 time[3];
> u8 tx_errors_count;
> u8 rx_errors_count;
> +
> u8 status;
> u8 padding[3];
> } __packed;
>
> -struct kvaser_msg_tx_acknowledge {
> +struct usbcan_msg_chip_state_event {
> + u8 tid;
> + u8 channel;
> +
> + u8 tx_errors_count;
> + u8 rx_errors_count;
> + __le16 time;
> +
> + u8 status;
> + u8 padding[3];
> +} __packed;
> +
> +struct kvaser_msg_tx_acknowledge_header {
> + u8 channel;
> + u8 tid;
> +};
> +
> +struct leaf_msg_tx_acknowledge {
> u8 channel;
> u8 tid;
> +
> __le16 time[3];
> u8 flags;
> u8 time_offset;
> } __packed;
>
> -struct kvaser_msg_error_event {
> +struct usbcan_msg_tx_acknowledge {
> + u8 channel;
> + u8 tid;
> +
> + __le16 time;
> + __le16 padding;
> +} __packed;
> +
> +struct leaf_msg_error_event {
> u8 tid;
> u8 flags;
> __le16 time[3];
> @@ -229,6 +346,18 @@ struct kvaser_msg_error_event {
> u8 error_factor;
> } __packed;
>
> +struct usbcan_msg_error_event {
> + u8 tid;
> + u8 padding;
> + u8 tx_errors_count_ch0;
> + u8 rx_errors_count_ch0;
> + u8 tx_errors_count_ch1;
> + u8 rx_errors_count_ch1;
> + u8 status_ch0;
> + u8 status_ch1;
> + __le16 time;
> +} __packed;
> +
> struct kvaser_msg_ctrl_mode {
> u8 tid;
> u8 channel;
> @@ -243,7 +372,7 @@ struct kvaser_msg_flush_queue {
> u8 padding[3];
> } __packed;
>
> -struct kvaser_msg_log_message {
> +struct leaf_msg_log_message {
> u8 channel;
> u8 flags;
> __le16 time[3];
> @@ -260,19 +389,49 @@ struct kvaser_msg {
> struct kvaser_msg_simple simple;
> struct kvaser_msg_cardinfo cardinfo;
> struct kvaser_msg_cardinfo2 cardinfo2;
> - struct kvaser_msg_softinfo softinfo;
> struct kvaser_msg_busparams busparams;
> +
> + struct kvaser_msg_rx_can_header rx_can_header;
> + struct kvaser_msg_tx_acknowledge_header tx_acknowledge_header;
> +
> + union {
> + struct leaf_msg_softinfo softinfo;
> + struct leaf_msg_rx_can rx_can;
> + struct leaf_msg_chip_state_event chip_state_event;
> + struct leaf_msg_tx_acknowledge tx_acknowledge;
> + struct leaf_msg_error_event error_event;
> + struct leaf_msg_log_message log_message;
> + } __packed leaf;
> +
> + union {
> + struct usbcan_msg_softinfo softinfo;
> + struct usbcan_msg_rx_can rx_can;
> + struct usbcan_msg_chip_state_event chip_state_event;
> + struct usbcan_msg_tx_acknowledge tx_acknowledge;
> + struct usbcan_msg_error_event error_event;
> + } __packed usbcan;
> +
> struct kvaser_msg_tx_can tx_can;
> - struct kvaser_msg_rx_can rx_can;
> - struct kvaser_msg_chip_state_event chip_state_event;
> - struct kvaser_msg_tx_acknowledge tx_acknowledge;
> - struct kvaser_msg_error_event error_event;
> struct kvaser_msg_ctrl_mode ctrl_mode;
> struct kvaser_msg_flush_queue flush_queue;
> - struct kvaser_msg_log_message log_message;
> } u;
> } __packed;
>
> +/* Leaf/USBCAN-agnostic summary of an error event.
> + * No M16C error factors for USBCAN-based devices. */
> +struct kvaser_error_summary {
> + u8 channel, status, txerr, rxerr;
> + union {
> + struct {
> + u8 error_factor;
> + } leaf;
> + struct {
> + u8 other_ch_status;
> + u8 error_state;
> + } usbcan;
> + };
> +};
> +
> struct kvaser_usb_tx_urb_context {
> struct kvaser_usb_net_priv *priv;
> u32 echo_index;
> @@ -288,6 +447,8 @@ struct kvaser_usb {
>
> u32 fw_version;
> unsigned int nchannels;
> + enum kvaser_usb_family family;
> + unsigned int max_channels;
>
> bool rxinitdone;
> void *rxbuf[MAX_RX_URBS];
> @@ -311,6 +472,7 @@ struct kvaser_usb_net_priv {
> };
>
> static const struct usb_device_id kvaser_usb_table[] = {
> + /* Leaf family IDs */
> { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
> { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
> { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
> @@ -360,6 +522,17 @@ static const struct usb_device_id kvaser_usb_table[] = {
> .driver_info = KVASER_HAS_TXRX_ERRORS },
> { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_V2_PRODUCT_ID) },
> { USB_DEVICE(KVASER_VENDOR_ID, USB_MINI_PCIE_HS_PRODUCT_ID) },
> +
> + /* USBCANII family IDs */
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN2_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN_REVB_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMORATOR_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_VCI2_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> +
> { }
> };
> MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
> @@ -463,7 +636,18 @@ static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
> if (err)
> return err;
>
> - dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
> + switch (dev->family) {
> + case KVASER_LEAF:
> + dev->fw_version = le32_to_cpu(msg.u.leaf.softinfo.fw_version);
> + break;
> + case KVASER_USBCAN:
> + dev->fw_version = le32_to_cpu(msg.u.usbcan.softinfo.fw_version);
> + break;
> + default:
> + dev_err(dev->udev->dev.parent,
> + "Invalid device family (%d)\n", dev->family);
> + return -EINVAL;
> + }
>
> return 0;
> }
> @@ -482,7 +666,7 @@ static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
> return err;
>
> dev->nchannels = msg.u.cardinfo.nchannels;
> - if (dev->nchannels > MAX_NET_DEVICES)
> + if (dev->nchannels > dev->max_channels)
> return -EINVAL;
IMHO, you can keep MAX_NET_DEVICES here.
>
> return 0;
> @@ -496,8 +680,10 @@ static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
> struct kvaser_usb_net_priv *priv;
> struct sk_buff *skb;
> struct can_frame *cf;
> - u8 channel = msg->u.tx_acknowledge.channel;
> - u8 tid = msg->u.tx_acknowledge.tid;
> + u8 channel, tid;
> +
> + channel = msg->u.tx_acknowledge_header.channel;
> + tid = msg->u.tx_acknowledge_header.tid;
>
> if (channel >= dev->nchannels) {
> dev_err(dev->udev->dev.parent,
> @@ -615,37 +801,83 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> }
>
> -static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> - const struct kvaser_msg *msg)
> +static void kvaser_report_error_event(const struct kvaser_usb *dev,
> + struct kvaser_error_summary *es);
> +
> +/*
> + * Report error to userspace iff the controller's errors counter has
> + * increased, or we're the only channel seeing the bus error state.
> + *
> + * As reported by USBCAN sheets, "the CAN controller has difficulties
> + * to tell whether an error frame arrived on channel 1 or on channel 2."
> + * Thus, error counters are compared with their earlier values to
> + * determine which channel was responsible for the error event.
> + */
> +static void usbcan_report_error_if_applicable(const struct kvaser_usb *dev,
> + struct kvaser_error_summary *es)
> {
> - struct can_frame *cf;
> - struct sk_buff *skb;
> - struct net_device_stats *stats;
> struct kvaser_usb_net_priv *priv;
> - unsigned int new_state;
> - u8 channel, status, txerr, rxerr, error_factor;
> + int old_tx_err_count, old_rx_err_count, channel, report_error;
> +
> + channel = es->channel;
> + if (channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", channel);
> + return;
> + }
> +
> + priv = dev->nets[channel];
> + old_tx_err_count = priv->bec.txerr;
> + old_rx_err_count = priv->bec.rxerr;
> +
> + report_error = 0;
> + if (es->txerr > old_tx_err_count) {
> + es->usbcan.error_state |= USBCAN_ERROR_STATE_TX_ERROR;
> + report_error = 1;
> + }
> + if (es->rxerr > old_rx_err_count) {
> + es->usbcan.error_state |= USBCAN_ERROR_STATE_RX_ERROR;
> + report_error = 1;
> + }
> + if ((es->status & M16C_STATE_BUS_ERROR) &&
> + !(es->usbcan.other_ch_status & M16C_STATE_BUS_ERROR)) {
> + es->usbcan.error_state |= USBCAN_ERROR_STATE_BUSERROR;
> + report_error = 1;
> + }
> +
> + if (report_error)
> + kvaser_report_error_event(dev, es);
> +}
> +
> +/*
> + * Extract error summary from a Leaf-based device error message
> + */
> +static void leaf_extract_error_from_msg(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct kvaser_error_summary es = { 0, };
>
> switch (msg->id) {
> case CMD_CAN_ERROR_EVENT:
> - channel = msg->u.error_event.channel;
> - status = msg->u.error_event.status;
> - txerr = msg->u.error_event.tx_errors_count;
> - rxerr = msg->u.error_event.rx_errors_count;
> - error_factor = msg->u.error_event.error_factor;
> + es.channel = msg->u.leaf.error_event.channel;
> + es.status = msg->u.leaf.error_event.status;
> + es.txerr = msg->u.leaf.error_event.tx_errors_count;
> + es.rxerr = msg->u.leaf.error_event.rx_errors_count;
> + es.leaf.error_factor = msg->u.leaf.error_event.error_factor;
> break;
> - case CMD_LOG_MESSAGE:
> - channel = msg->u.log_message.channel;
> - status = msg->u.log_message.data[0];
> - txerr = msg->u.log_message.data[2];
> - rxerr = msg->u.log_message.data[3];
> - error_factor = msg->u.log_message.data[1];
> + case LEAF_CMD_LOG_MESSAGE:
> + es.channel = msg->u.leaf.log_message.channel;
> + es.status = msg->u.leaf.log_message.data[0];
> + es.txerr = msg->u.leaf.log_message.data[2];
> + es.rxerr = msg->u.leaf.log_message.data[3];
> + es.leaf.error_factor = msg->u.leaf.log_message.data[1];
> break;
> case CMD_CHIP_STATE_EVENT:
> - channel = msg->u.chip_state_event.channel;
> - status = msg->u.chip_state_event.status;
> - txerr = msg->u.chip_state_event.tx_errors_count;
> - rxerr = msg->u.chip_state_event.rx_errors_count;
> - error_factor = 0;
> + es.channel = msg->u.leaf.chip_state_event.channel;
> + es.status = msg->u.leaf.chip_state_event.status;
> + es.txerr = msg->u.leaf.chip_state_event.tx_errors_count;
> + es.rxerr = msg->u.leaf.chip_state_event.rx_errors_count;
> + es.leaf.error_factor = 0;
> break;
> default:
> dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> @@ -653,16 +885,92 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> return;
> }
>
> - if (channel >= dev->nchannels) {
> + kvaser_report_error_event(dev, &es);
> +}
> +
> +/*
> + * Extract summary from a USBCANII-based device error message.
> + */
> +static void usbcan_extract_error_from_msg(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct kvaser_error_summary es = { 0, };
> +
> + switch (msg->id) {
> +
> + /* Sometimes errors are sent as unsolicited chip state events */
> + case CMD_CHIP_STATE_EVENT:
> + es.channel = msg->u.usbcan.chip_state_event.channel;
> + es.status = msg->u.usbcan.chip_state_event.status;
> + es.txerr = msg->u.usbcan.chip_state_event.tx_errors_count;
> + es.rxerr = msg->u.usbcan.chip_state_event.rx_errors_count;
> + usbcan_report_error_if_applicable(dev, &es);
> + break;
> +
> + case CMD_CAN_ERROR_EVENT:
> + es.channel = 0;
> + es.status = msg->u.usbcan.error_event.status_ch0;
> + es.txerr = msg->u.usbcan.error_event.tx_errors_count_ch0;
> + es.rxerr = msg->u.usbcan.error_event.rx_errors_count_ch0;
> + es.usbcan.other_ch_status =
> + msg->u.usbcan.error_event.status_ch1;
> + usbcan_report_error_if_applicable(dev, &es);
> +
> + /* For error events, the USBCAN firmware does not support
> + * more than 2 channels: ch0, and ch1. */
> + if (dev->nchannels > 1) {
> + es.channel = 1;
> + es.status = msg->u.usbcan.error_event.status_ch1;
> + es.txerr = msg->u.usbcan.error_event.tx_errors_count_ch1;
> + es.rxerr = msg->u.usbcan.error_event.rx_errors_count_ch1;
> + es.usbcan.other_ch_status =
> + msg->u.usbcan.error_event.status_ch0;
> + usbcan_report_error_if_applicable(dev, &es);
> + }
> + break;
> +
> + default:
> + dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> + msg->id);
> + }
> +}
> +
> +static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + switch (dev->family) {
> + case KVASER_LEAF:
> + leaf_extract_error_from_msg(dev, msg);
> + break;
> + case KVASER_USBCAN:
> + usbcan_extract_error_from_msg(dev, msg);
> + break;
> + default:
> dev_err(dev->udev->dev.parent,
> - "Invalid channel number (%d)\n", channel);
> + "Invalid device family (%d)\n", dev->family);
> return;
> }
> +}
>
> - priv = dev->nets[channel];
> +static void kvaser_report_error_event(const struct kvaser_usb *dev,
> + struct kvaser_error_summary *es)
> +{
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + struct net_device_stats *stats;
> + struct kvaser_usb_net_priv *priv;
> + unsigned int new_state;
> +
> + if (es->channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", es->channel);
> + return;
> + }
> +
> + priv = dev->nets[es->channel];
> stats = &priv->netdev->stats;
>
> - if (status & M16C_STATE_BUS_RESET) {
> + if (es->status & M16C_STATE_BUS_RESET) {
> kvaser_usb_unlink_tx_urbs(priv);
> return;
> }
> @@ -675,9 +983,9 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
>
> new_state = priv->can.state;
>
> - netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
> + netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
>
> - if (status & M16C_STATE_BUS_OFF) {
> + if (es->status & M16C_STATE_BUS_OFF) {
> cf->can_id |= CAN_ERR_BUSOFF;
>
> priv->can.can_stats.bus_off++;
> @@ -687,12 +995,12 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> netif_carrier_off(priv->netdev);
>
> new_state = CAN_STATE_BUS_OFF;
> - } else if (status & M16C_STATE_BUS_PASSIVE) {
> + } else if (es->status & M16C_STATE_BUS_PASSIVE) {
> if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
> cf->can_id |= CAN_ERR_CRTL;
>
> - if (txerr || rxerr)
> - cf->data[1] = (txerr > rxerr)
> + if (es->txerr || es->rxerr)
> + cf->data[1] = (es->txerr > es->rxerr)
> ? CAN_ERR_CRTL_TX_PASSIVE
> : CAN_ERR_CRTL_RX_PASSIVE;
> else
> @@ -703,13 +1011,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> }
>
> new_state = CAN_STATE_ERROR_PASSIVE;
> - }
> -
> - if (status == M16C_STATE_BUS_ERROR) {
> + } else if (es->status & M16C_STATE_BUS_ERROR) {
> if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> - ((txerr >= 96) || (rxerr >= 96))) {
> + ((es->txerr >= 96) || (es->rxerr >= 96))) {
> cf->can_id |= CAN_ERR_CRTL;
> - cf->data[1] = (txerr > rxerr)
> + cf->data[1] = (es->txerr > es->rxerr)
> ? CAN_ERR_CRTL_TX_WARNING
> : CAN_ERR_CRTL_RX_WARNING;
>
> @@ -723,7 +1029,7 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> }
> }
>
> - if (!status) {
> + if (!es->status) {
> cf->can_id |= CAN_ERR_PROT;
> cf->data[2] = CAN_ERR_PROT_ACTIVE;
>
> @@ -739,34 +1045,52 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> priv->can.can_stats.restarts++;
> }
>
> - if (error_factor) {
> - priv->can.can_stats.bus_error++;
> - stats->rx_errors++;
> -
> - cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> -
> - if (error_factor & M16C_EF_ACKE)
> - cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
> - if (error_factor & M16C_EF_CRCE)
> - cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> - CAN_ERR_PROT_LOC_CRC_DEL);
> - if (error_factor & M16C_EF_FORME)
> - cf->data[2] |= CAN_ERR_PROT_FORM;
> - if (error_factor & M16C_EF_STFE)
> - cf->data[2] |= CAN_ERR_PROT_STUFF;
> - if (error_factor & M16C_EF_BITE0)
> - cf->data[2] |= CAN_ERR_PROT_BIT0;
> - if (error_factor & M16C_EF_BITE1)
> - cf->data[2] |= CAN_ERR_PROT_BIT1;
> - if (error_factor & M16C_EF_TRE)
> - cf->data[2] |= CAN_ERR_PROT_TX;
> + switch (dev->family) {
> + case KVASER_LEAF:
> + if (es->leaf.error_factor) {
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> +
> + cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> +
> + if (es->leaf.error_factor & M16C_EF_ACKE)
> + cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
> + if (es->leaf.error_factor & M16C_EF_CRCE)
> + cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> + CAN_ERR_PROT_LOC_CRC_DEL);
> + if (es->leaf.error_factor & M16C_EF_FORME)
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + if (es->leaf.error_factor & M16C_EF_STFE)
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> + if (es->leaf.error_factor & M16C_EF_BITE0)
> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> + if (es->leaf.error_factor & M16C_EF_BITE1)
> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> + if (es->leaf.error_factor & M16C_EF_TRE)
> + cf->data[2] |= CAN_ERR_PROT_TX;
> + }
> + break;
> + case KVASER_USBCAN:
> + if (es->usbcan.error_state & USBCAN_ERROR_STATE_TX_ERROR)
> + stats->tx_errors++;
> + if (es->usbcan.error_state & USBCAN_ERROR_STATE_RX_ERROR)
> + stats->rx_errors++;
> + if (es->usbcan.error_state & USBCAN_ERROR_STATE_BUSERROR) {
> + priv->can.can_stats.bus_error++;
> + cf->can_id |= CAN_ERR_BUSERROR;
> + }
> + break;
> + default:
> + dev_err(dev->udev->dev.parent,
> + "Invalid device family (%d)\n", dev->family);
> + goto err;
> }
>
> - cf->data[6] = txerr;
> - cf->data[7] = rxerr;
> + cf->data[6] = es->txerr;
> + cf->data[7] = es->rxerr;
>
> - priv->bec.txerr = txerr;
> - priv->bec.rxerr = rxerr;
> + priv->bec.txerr = es->txerr;
> + priv->bec.rxerr = es->rxerr;
>
> priv->can.state = new_state;
>
> @@ -774,6 +1098,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
>
> stats->rx_packets++;
> stats->rx_bytes += cf->can_dlc;
> +
> + return;
> +
> +err:
> + dev_kfree_skb(skb);
> }
>
> static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> @@ -783,16 +1112,16 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> struct sk_buff *skb;
> struct net_device_stats *stats = &priv->netdev->stats;
>
> - if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> + if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
> MSG_FLAG_NERR)) {
> netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n",
> - msg->u.rx_can.flag);
> + msg->u.rx_can_header.flag);
>
> stats->rx_errors++;
> return;
> }
>
> - if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
> + if (msg->u.rx_can_header.flag & MSG_FLAG_OVERRUN) {
> skb = alloc_can_err_skb(priv->netdev, &cf);
> if (!skb) {
> stats->rx_dropped++;
> @@ -819,7 +1148,8 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> struct can_frame *cf;
> struct sk_buff *skb;
> struct net_device_stats *stats;
> - u8 channel = msg->u.rx_can.channel;
> + u8 channel = msg->u.rx_can_header.channel;
> + const u8 *rx_msg;
>
> if (channel >= dev->nchannels) {
> dev_err(dev->udev->dev.parent,
> @@ -830,19 +1160,32 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> priv = dev->nets[channel];
> stats = &priv->netdev->stats;
>
> - if ((msg->u.rx_can.flag & MSG_FLAG_ERROR_FRAME) &&
> - (msg->id == CMD_LOG_MESSAGE)) {
> + if ((msg->u.rx_can_header.flag & MSG_FLAG_ERROR_FRAME) &&
> + (dev->family == KVASER_LEAF && msg->id == LEAF_CMD_LOG_MESSAGE)) {
> kvaser_usb_rx_error(dev, msg);
> return;
> - } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> - MSG_FLAG_NERR |
> - MSG_FLAG_OVERRUN)) {
> + } else if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
> + MSG_FLAG_NERR |
> + MSG_FLAG_OVERRUN)) {
> kvaser_usb_rx_can_err(priv, msg);
> return;
> - } else if (msg->u.rx_can.flag & ~MSG_FLAG_REMOTE_FRAME) {
> + } else if (msg->u.rx_can_header.flag & ~MSG_FLAG_REMOTE_FRAME) {
> netdev_warn(priv->netdev,
> "Unhandled frame (flags: 0x%02x)",
> - msg->u.rx_can.flag);
> + msg->u.rx_can_header.flag);
> + return;
> + }
> +
> + switch (dev->family) {
> + case KVASER_LEAF:
> + rx_msg = msg->u.leaf.rx_can.msg;
> + break;
> + case KVASER_USBCAN:
> + rx_msg = msg->u.usbcan.rx_can.msg;
> + break;
> + default:
> + dev_err(dev->udev->dev.parent,
> + "Invalid device family (%d)\n", dev->family);
> return;
> }
>
> @@ -852,38 +1195,37 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> return;
> }
>
> - if (msg->id == CMD_LOG_MESSAGE) {
> - cf->can_id = le32_to_cpu(msg->u.log_message.id);
> + if (dev->family == KVASER_LEAF && msg->id == LEAF_CMD_LOG_MESSAGE) {
> + cf->can_id = le32_to_cpu(msg->u.leaf.log_message.id);
> if (cf->can_id & KVASER_EXTENDED_FRAME)
> cf->can_id &= CAN_EFF_MASK | CAN_EFF_FLAG;
> else
> cf->can_id &= CAN_SFF_MASK;
>
> - cf->can_dlc = get_can_dlc(msg->u.log_message.dlc);
> + cf->can_dlc = get_can_dlc(msg->u.leaf.log_message.dlc);
>
> - if (msg->u.log_message.flags & MSG_FLAG_REMOTE_FRAME)
> + if (msg->u.leaf.log_message.flags & MSG_FLAG_REMOTE_FRAME)
> cf->can_id |= CAN_RTR_FLAG;
> else
> - memcpy(cf->data, &msg->u.log_message.data,
> + memcpy(cf->data, &msg->u.leaf.log_message.data,
> cf->can_dlc);
> } else {
> - cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
> - (msg->u.rx_can.msg[1] & 0x3f);
> + cf->can_id = ((rx_msg[0] & 0x1f) << 6) | (rx_msg[1] & 0x3f);
>
> if (msg->id == CMD_RX_EXT_MESSAGE) {
> cf->can_id <<= 18;
> - cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
> - ((msg->u.rx_can.msg[3] & 0xff) << 6) |
> - (msg->u.rx_can.msg[4] & 0x3f);
> + cf->can_id |= ((rx_msg[2] & 0x0f) << 14) |
> + ((rx_msg[3] & 0xff) << 6) |
> + (rx_msg[4] & 0x3f);
> cf->can_id |= CAN_EFF_FLAG;
> }
>
> - cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
> + cf->can_dlc = get_can_dlc(rx_msg[5]);
>
> - if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME)
> + if (msg->u.rx_can_header.flag & MSG_FLAG_REMOTE_FRAME)
> cf->can_id |= CAN_RTR_FLAG;
> else
> - memcpy(cf->data, &msg->u.rx_can.msg[6],
> + memcpy(cf->data, &rx_msg[6],
> cf->can_dlc);
> }
>
> @@ -947,7 +1289,12 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
>
> case CMD_RX_STD_MESSAGE:
> case CMD_RX_EXT_MESSAGE:
> - case CMD_LOG_MESSAGE:
> + kvaser_usb_rx_can_msg(dev, msg);
> + break;
> +
> + case LEAF_CMD_LOG_MESSAGE:
> + if (dev->family != KVASER_LEAF)
> + goto warn;
> kvaser_usb_rx_can_msg(dev, msg);
> break;
>
> @@ -960,8 +1307,14 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
> kvaser_usb_tx_acknowledge(dev, msg);
> break;
>
> + /* Ignored messages */
> + case USBCAN_CMD_CLOCK_OVERFLOW_EVENT:
> + if (dev->family != KVASER_USBCAN)
> + goto warn;
> + break;
> +
> default:
> - dev_warn(dev->udev->dev.parent,
> +warn: dev_warn(dev->udev->dev.parent,
> "Unhandled message (%d)\n", msg->id);
> break;
> }
> @@ -1181,7 +1534,7 @@ static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
> dev->rxbuf[i],
> dev->rxbuf_dma[i]);
>
> - for (i = 0; i < MAX_NET_DEVICES; i++) {
> + for (i = 0; i < dev->max_channels; i++) {
here too... or replace it by nchannels.
> struct kvaser_usb_net_priv *priv = dev->nets[i];
>
> if (priv)
> @@ -1286,6 +1639,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> struct kvaser_msg *msg;
> int i, err;
> int ret = NETDEV_TX_OK;
> + uint8_t *msg_tx_can_flags;
> bool kfree_skb_on_error = true;
>
> if (can_dropped_invalid_skb(netdev, skb))
> @@ -1306,9 +1660,23 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
>
> msg = buf;
> msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
> - msg->u.tx_can.flags = 0;
> msg->u.tx_can.channel = priv->channel;
>
> + switch (dev->family) {
> + case KVASER_LEAF:
> + msg_tx_can_flags = &msg->u.tx_can.leaf.flags;
> + break;
> + case KVASER_USBCAN:
> + msg_tx_can_flags = &msg->u.tx_can.usbcan.flags;
> + break;
> + default:
> + dev_err(dev->udev->dev.parent,
> + "Invalid device family (%d)\n", dev->family);
> + goto releasebuf;
> + }
> +
> + *msg_tx_can_flags = 0;
> +
> if (cf->can_id & CAN_EFF_FLAG) {
> msg->id = CMD_TX_EXT_MESSAGE;
> msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
> @@ -1326,7 +1694,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc);
>
> if (cf->can_id & CAN_RTR_FLAG)
> - msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME;
> + *msg_tx_can_flags |= MSG_FLAG_REMOTE_FRAME;
>
> for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) {
> if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
> @@ -1596,6 +1964,18 @@ static int kvaser_usb_probe(struct usb_interface *intf,
> if (!dev)
> return -ENOMEM;
>
> + if (LEAF_PRODUCT_ID(id->idProduct)) {
> + dev->family = KVASER_LEAF;
> + dev->max_channels = LEAF_MAX_NET_DEVICES;
> + } else if (USBCAN_PRODUCT_ID(id->idProduct)) {
> + dev->family = KVASER_USBCAN;
> + dev->max_channels = USBCAN_MAX_NET_DEVICES;
> + } else {
> + dev_err(&intf->dev, "Product ID (%d) does not belong to any "
> + "known Kvaser USB family", id->idProduct);
> + return -ENODEV;
> + }
Is it really required to keep max_channels in the kvaser_usb structure?
If I looked correctly, you use this variable as a replacement for
MAX_NET_DEVICES in the code and MAX_NET_DEVICES is only used in probe
and disconnect functions. I think it can even be replaced by nchannels
in the disconnect path. So I also think that it don't need to be in the
kvaser_usb structure.
> +
> err = kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
> if (err) {
> dev_err(&intf->dev, "Cannot get usb endpoint(s)");
> @@ -1608,7 +1988,7 @@ static int kvaser_usb_probe(struct usb_interface *intf,
>
> usb_set_intfdata(intf, dev);
>
> - for (i = 0; i < MAX_NET_DEVICES; i++)
> + for (i = 0; i < dev->max_channels; i++)
> kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i);
Someone reported me that recent leaf firmwares go in trouble when
you send a command for a channel that does not exist. Instead of
max_channels, you can use nchannels here and move the reset command
in the kvaser_usb_init_one() function.
I've a patch for this but It is not tested yet. I'll send it next week-end after
I did some tests.
>
> err = kvaser_usb_get_software_info(dev);
Thank you,
--
Olivier
^ permalink raw reply
* Re: [PATCH] can: kvaser_usb: Don't free packets when tight on URBs
From: Olivier Sobrie @ 2014-12-24 12:31 UTC (permalink / raw)
To: Ahmed S. Darwish
Cc: Oliver Hartkopp, Wolfgang Grandegger, Marc Kleine-Budde,
David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20141223154654.GB6460@vivalin-002>
Hello Ahmed,
On Tue, Dec 23, 2014 at 05:46:54PM +0200, Ahmed S. Darwish wrote:
> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>
> Flooding the Kvaser CAN to USB dongle with multiple reads and
> writes in high frequency caused seemingly-random panics in the
> kernel.
>
> On further inspection, it seems the driver erroneously freed the
> to-be-transmitted packet upon getting tight on URBs and returning
> NETDEV_TX_BUSY, leading to invalid memory writes and double frees
> at a later point in time.
>
> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Thank you for the fix.
> ---
> drivers/net/can/usb/kvaser_usb.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> (Generated over 3.19.0-rc1)
>
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> index 541fb7a..34c35d8 100644
> --- a/drivers/net/can/usb/kvaser_usb.c
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -1286,6 +1286,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> struct kvaser_msg *msg;
> int i, err;
> int ret = NETDEV_TX_OK;
> + bool kfree_skb_on_error = true;
>
> if (can_dropped_invalid_skb(netdev, skb))
> return NETDEV_TX_OK;
> @@ -1336,6 +1337,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
>
> if (!context) {
> netdev_warn(netdev, "cannot find free context\n");
> + kfree_skb_on_error = false;
Instead of using an extra variable, you can also set skb to NULL here.
Or maybe better, you can move the dev_kfree_skb() in the two previous
tests (in the check of variables urb and buf).
Thank you,
Olivier
> ret = NETDEV_TX_BUSY;
> goto releasebuf;
> }
> @@ -1364,8 +1366,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> if (unlikely(err)) {
> can_free_echo_skb(netdev, context->echo_index);
>
> - skb = NULL; /* set to NULL to avoid double free in
> - * dev_kfree_skb(skb) */
> + kfree_skb_on_error = false;
>
> atomic_dec(&priv->active_tx_urbs);
> usb_unanchor_urb(urb);
> @@ -1389,7 +1390,8 @@ releasebuf:
> nobufmem:
> usb_free_urb(urb);
> nourbmem:
> - dev_kfree_skb(skb);
> + if (kfree_skb_on_error)
> + dev_kfree_skb(skb);
> return ret;
> }
>
^ permalink raw reply
* Re: [PATCH 2/3] net/fsl: remove irq assignment from xgmac_mdio
From: Sergei Shtylyov @ 2014-12-24 10:51 UTC (permalink / raw)
To: Shaohui Xie
Cc: shh.xie@gmail.com, netdev@vger.kernel.org, davem@davemloft.net
In-Reply-To: <DM2PR0301MB08643B456EB87328ACF71CC5E2540@DM2PR0301MB0864.namprd03.prod.outlook.com>
Hello.
On 12/24/2014 5:22 AM, Shaohui Xie wrote:
[...]
>>> From: Shaohui Xie <Shaohui.Xie@freescale.com>
>>> Which is wrong and not used, so no extra space needed by
>>> mdio_alloc_size(), change the parameter accordingly.
>>> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
>>> ---
>>> drivers/net/ethernet/freescale/xgmac_mdio.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>> diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c
>>> b/drivers/net/ethernet/freescale/xgmac_mdio.c
>>> index 90adba1..72e0b85 100644
>>> --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
>>> +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
>>> @@ -187,14 +187,13 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
>>> return ret;
>>> }
>>>
>>> - bus = mdiobus_alloc_size(PHY_MAX_ADDR * sizeof(int));
>>> + bus = mdiobus_alloc_size(0);
>> It's now equivalent to a mere mdiobus_alloc().
> [S.H] Yes, mdiobus_alloc() defined as:
> static inline struct mii_bus *mdiobus_alloc(void)
> {
> return mdiobus_alloc_size(0);
> }
> Should I use mdiobus_alloc() instead?
Yes, I meant it.
> Thanks!
> Shaohui
WBR, Sergei
^ permalink raw reply
* [PATCH net-next 0/3] enic: Check for DMA mapping error
From: Govindarajulu Varadarajan @ 2014-12-24 10:29 UTC (permalink / raw)
To: davem, netdev, sassmann; +Cc: ssujith, benve, Govindarajulu Varadarajan
After dma mapping the buffers, enic does not call dma_mapping_error() to check
if mapping is successful.
This series fixes the issue by checking return value of pci_dma_mapping_error()
after pci_map_single().
This is reported by redhat here
https://bugzilla.redhat.com/show_bug.cgi?id=1145016
Govindarajulu Varadarajan (3):
enic: make vnic_wq_buf doubly linked
enic: check dma_mapping_error
enic: add stats for dma mapping error
drivers/net/ethernet/cisco/enic/enic.h | 14 +++
drivers/net/ethernet/cisco/enic/enic_ethtool.c | 19 +++-
drivers/net/ethernet/cisco/enic/enic_main.c | 152 +++++++++++++++----------
drivers/net/ethernet/cisco/enic/vnic_stats.h | 5 +
drivers/net/ethernet/cisco/enic/vnic_wq.c | 3 +
drivers/net/ethernet/cisco/enic/vnic_wq.h | 1 +
6 files changed, 135 insertions(+), 59 deletions(-)
--
2.2.1
^ permalink raw reply
* [PATCH net-next 3/3] enic: add stats for dma mapping error
From: Govindarajulu Varadarajan @ 2014-12-24 10:29 UTC (permalink / raw)
To: davem, netdev, sassmann; +Cc: ssujith, benve, Govindarajulu Varadarajan
In-Reply-To: <1419416978-1008-1-git-send-email-_govind@gmx.com>
This patch adds generic statistics for enic. As of now dma_map_error is the only
member. dma_map_erro is incremented every time dma maping error happens.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/cisco/enic/enic.h | 2 ++
drivers/net/ethernet/cisco/enic/enic_ethtool.c | 19 ++++++++++++++++++-
drivers/net/ethernet/cisco/enic/vnic_stats.h | 5 +++++
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index b2ea35a..d2a1035 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -188,6 +188,7 @@ struct enic {
struct enic_rfs_flw_tbl rfs_h;
u32 rx_copybreak;
u8 rss_key[ENIC_RSS_LEN];
+ struct vnic_gen_stats gen_stats;
};
static inline struct device *enic_get_dev(struct enic *enic)
@@ -247,6 +248,7 @@ static inline int enic_dma_map_check(struct enic *enic, dma_addr_t dma_addr)
if (unlikely(pci_dma_mapping_error(enic->pdev, dma_addr))) {
net_warn_ratelimited("%s: PCI dma mapping failed!\n",
enic->netdev->name);
+ enic->gen_stats.dma_map_error++;
return -ENOMEM;
}
diff --git a/drivers/net/ethernet/cisco/enic/enic_ethtool.c b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
index eba1eb8..0c396c1 100644
--- a/drivers/net/ethernet/cisco/enic/enic_ethtool.c
+++ b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
@@ -24,6 +24,7 @@
#include "enic_dev.h"
#include "enic_clsf.h"
#include "vnic_rss.h"
+#include "vnic_stats.h"
struct enic_stat {
char name[ETH_GSTRING_LEN];
@@ -40,6 +41,11 @@ struct enic_stat {
.index = offsetof(struct vnic_rx_stats, stat) / sizeof(u64) \
}
+#define ENIC_GEN_STAT(stat) { \
+ .name = #stat, \
+ .index = offsetof(struct vnic_gen_stats, stat) / sizeof(u64)\
+}
+
static const struct enic_stat enic_tx_stats[] = {
ENIC_TX_STAT(tx_frames_ok),
ENIC_TX_STAT(tx_unicast_frames_ok),
@@ -78,8 +84,13 @@ static const struct enic_stat enic_rx_stats[] = {
ENIC_RX_STAT(rx_frames_to_max),
};
+static const struct enic_stat enic_gen_stats[] = {
+ ENIC_GEN_STAT(dma_map_error),
+};
+
static const unsigned int enic_n_tx_stats = ARRAY_SIZE(enic_tx_stats);
static const unsigned int enic_n_rx_stats = ARRAY_SIZE(enic_rx_stats);
+static const unsigned int enic_n_gen_stats = ARRAY_SIZE(enic_gen_stats);
void enic_intr_coal_set_rx(struct enic *enic, u32 timer)
{
@@ -146,6 +157,10 @@ static void enic_get_strings(struct net_device *netdev, u32 stringset,
memcpy(data, enic_rx_stats[i].name, ETH_GSTRING_LEN);
data += ETH_GSTRING_LEN;
}
+ for (i = 0; i < enic_n_gen_stats; i++) {
+ memcpy(data, enic_gen_stats[i].name, ETH_GSTRING_LEN);
+ data += ETH_GSTRING_LEN;
+ }
break;
}
}
@@ -154,7 +169,7 @@ static int enic_get_sset_count(struct net_device *netdev, int sset)
{
switch (sset) {
case ETH_SS_STATS:
- return enic_n_tx_stats + enic_n_rx_stats;
+ return enic_n_tx_stats + enic_n_rx_stats + enic_n_gen_stats;
default:
return -EOPNOTSUPP;
}
@@ -173,6 +188,8 @@ static void enic_get_ethtool_stats(struct net_device *netdev,
*(data++) = ((u64 *)&vstats->tx)[enic_tx_stats[i].index];
for (i = 0; i < enic_n_rx_stats; i++)
*(data++) = ((u64 *)&vstats->rx)[enic_rx_stats[i].index];
+ for (i = 0; i < enic_n_gen_stats; i++)
+ *(data++) = ((u64 *)&enic->gen_stats)[enic_gen_stats[i].index];
}
static u32 enic_get_msglevel(struct net_device *netdev)
diff --git a/drivers/net/ethernet/cisco/enic/vnic_stats.h b/drivers/net/ethernet/cisco/enic/vnic_stats.h
index 77750ec..74c81ed 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_stats.h
+++ b/drivers/net/ethernet/cisco/enic/vnic_stats.h
@@ -62,6 +62,11 @@ struct vnic_rx_stats {
u64 rsvd[16];
};
+/* Generic statistics */
+struct vnic_gen_stats {
+ u64 dma_map_error;
+};
+
struct vnic_stats {
struct vnic_tx_stats tx;
struct vnic_rx_stats rx;
--
2.2.1
^ permalink raw reply related
* [PATCH net-next 2/3] enic: check dma_mapping_error
From: Govindarajulu Varadarajan @ 2014-12-24 10:29 UTC (permalink / raw)
To: davem, netdev, sassmann; +Cc: ssujith, benve, Govindarajulu Varadarajan
In-Reply-To: <1419416978-1008-1-git-send-email-_govind@gmx.com>
This patch checks for pci_dma_mapping_error() after dma mapping the data.
If the dma mapping fails we remove the previously queued frags and return
NETDEV_TX_OK.
Reported-by: Jan Stancek <jstancek@redhat.com>
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/cisco/enic/enic.h | 12 +++
drivers/net/ethernet/cisco/enic/enic_main.c | 152 +++++++++++++++++-----------
2 files changed, 106 insertions(+), 58 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 25c4d88..b2ea35a 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -242,6 +242,18 @@ static inline unsigned int enic_msix_notify_intr(struct enic *enic)
return enic->rq_count + enic->wq_count + 1;
}
+static inline int enic_dma_map_check(struct enic *enic, dma_addr_t dma_addr)
+{
+ if (unlikely(pci_dma_mapping_error(enic->pdev, dma_addr))) {
+ net_warn_ratelimited("%s: PCI dma mapping failed!\n",
+ enic->netdev->name);
+
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
void enic_reset_addr_lists(struct enic *enic);
int enic_sriov_enabled(struct enic *enic);
int enic_is_valid_vf(struct enic *enic, int vf);
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 868d0f6..95c4804 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -351,80 +351,94 @@ static irqreturn_t enic_isr_msix_notify(int irq, void *data)
return IRQ_HANDLED;
}
-static inline void enic_queue_wq_skb_cont(struct enic *enic,
- struct vnic_wq *wq, struct sk_buff *skb,
- unsigned int len_left, int loopback)
+static int enic_queue_wq_skb_cont(struct enic *enic, struct vnic_wq *wq,
+ struct sk_buff *skb, unsigned int len_left,
+ int loopback)
{
const skb_frag_t *frag;
+ dma_addr_t dma_addr;
/* Queue additional data fragments */
for (frag = skb_shinfo(skb)->frags; len_left; frag++) {
len_left -= skb_frag_size(frag);
- enic_queue_wq_desc_cont(wq, skb,
- skb_frag_dma_map(&enic->pdev->dev,
- frag, 0, skb_frag_size(frag),
- DMA_TO_DEVICE),
- skb_frag_size(frag),
- (len_left == 0), /* EOP? */
- loopback);
+ dma_addr = skb_frag_dma_map(&enic->pdev->dev, frag, 0,
+ skb_frag_size(frag),
+ DMA_TO_DEVICE);
+ if (unlikely(enic_dma_map_check(enic, dma_addr)))
+ return -ENOMEM;
+ enic_queue_wq_desc_cont(wq, skb, dma_addr, skb_frag_size(frag),
+ (len_left == 0), /* EOP? */
+ loopback);
}
+
+ return 0;
}
-static inline void enic_queue_wq_skb_vlan(struct enic *enic,
- struct vnic_wq *wq, struct sk_buff *skb,
- int vlan_tag_insert, unsigned int vlan_tag, int loopback)
+static int enic_queue_wq_skb_vlan(struct enic *enic, struct vnic_wq *wq,
+ struct sk_buff *skb, int vlan_tag_insert,
+ unsigned int vlan_tag, int loopback)
{
unsigned int head_len = skb_headlen(skb);
unsigned int len_left = skb->len - head_len;
int eop = (len_left == 0);
+ dma_addr_t dma_addr;
+ int err = 0;
+
+ dma_addr = pci_map_single(enic->pdev, skb->data, head_len,
+ PCI_DMA_TODEVICE);
+ if (unlikely(enic_dma_map_check(enic, dma_addr)))
+ return -ENOMEM;
/* Queue the main skb fragment. The fragments are no larger
* than max MTU(9000)+ETH_HDR_LEN(14) bytes, which is less
* than WQ_ENET_MAX_DESC_LEN length. So only one descriptor
* per fragment is queued.
*/
- enic_queue_wq_desc(wq, skb,
- pci_map_single(enic->pdev, skb->data,
- head_len, PCI_DMA_TODEVICE),
- head_len,
- vlan_tag_insert, vlan_tag,
- eop, loopback);
+ enic_queue_wq_desc(wq, skb, dma_addr, head_len, vlan_tag_insert,
+ vlan_tag, eop, loopback);
if (!eop)
- enic_queue_wq_skb_cont(enic, wq, skb, len_left, loopback);
+ err = enic_queue_wq_skb_cont(enic, wq, skb, len_left, loopback);
+
+ return err;
}
-static inline void enic_queue_wq_skb_csum_l4(struct enic *enic,
- struct vnic_wq *wq, struct sk_buff *skb,
- int vlan_tag_insert, unsigned int vlan_tag, int loopback)
+static int enic_queue_wq_skb_csum_l4(struct enic *enic, struct vnic_wq *wq,
+ struct sk_buff *skb, int vlan_tag_insert,
+ unsigned int vlan_tag, int loopback)
{
unsigned int head_len = skb_headlen(skb);
unsigned int len_left = skb->len - head_len;
unsigned int hdr_len = skb_checksum_start_offset(skb);
unsigned int csum_offset = hdr_len + skb->csum_offset;
int eop = (len_left == 0);
+ dma_addr_t dma_addr;
+ int err = 0;
+
+ dma_addr = pci_map_single(enic->pdev, skb->data, head_len,
+ PCI_DMA_TODEVICE);
+ if (unlikely(enic_dma_map_check(enic, dma_addr)))
+ return -ENOMEM;
/* Queue the main skb fragment. The fragments are no larger
* than max MTU(9000)+ETH_HDR_LEN(14) bytes, which is less
* than WQ_ENET_MAX_DESC_LEN length. So only one descriptor
* per fragment is queued.
*/
- enic_queue_wq_desc_csum_l4(wq, skb,
- pci_map_single(enic->pdev, skb->data,
- head_len, PCI_DMA_TODEVICE),
- head_len,
- csum_offset,
- hdr_len,
- vlan_tag_insert, vlan_tag,
- eop, loopback);
+ enic_queue_wq_desc_csum_l4(wq, skb, dma_addr, head_len, csum_offset,
+ hdr_len, vlan_tag_insert, vlan_tag, eop,
+ loopback);
if (!eop)
- enic_queue_wq_skb_cont(enic, wq, skb, len_left, loopback);
+ err = enic_queue_wq_skb_cont(enic, wq, skb, len_left, loopback);
+
+ return err;
}
-static inline void enic_queue_wq_skb_tso(struct enic *enic,
- struct vnic_wq *wq, struct sk_buff *skb, unsigned int mss,
- int vlan_tag_insert, unsigned int vlan_tag, int loopback)
+static int enic_queue_wq_skb_tso(struct enic *enic, struct vnic_wq *wq,
+ struct sk_buff *skb, unsigned int mss,
+ int vlan_tag_insert, unsigned int vlan_tag,
+ int loopback)
{
unsigned int frag_len_left = skb_headlen(skb);
unsigned int len_left = skb->len - frag_len_left;
@@ -454,20 +468,19 @@ static inline void enic_queue_wq_skb_tso(struct enic *enic,
*/
while (frag_len_left) {
len = min(frag_len_left, (unsigned int)WQ_ENET_MAX_DESC_LEN);
- dma_addr = pci_map_single(enic->pdev, skb->data + offset,
- len, PCI_DMA_TODEVICE);
- enic_queue_wq_desc_tso(wq, skb,
- dma_addr,
- len,
- mss, hdr_len,
- vlan_tag_insert, vlan_tag,
- eop && (len == frag_len_left), loopback);
+ dma_addr = pci_map_single(enic->pdev, skb->data + offset, len,
+ PCI_DMA_TODEVICE);
+ if (unlikely(enic_dma_map_check(enic, dma_addr)))
+ return -ENOMEM;
+ enic_queue_wq_desc_tso(wq, skb, dma_addr, len, mss, hdr_len,
+ vlan_tag_insert, vlan_tag,
+ eop && (len == frag_len_left), loopback);
frag_len_left -= len;
offset += len;
}
if (eop)
- return;
+ return 0;
/* Queue WQ_ENET_MAX_DESC_LEN length descriptors
* for additional data fragments
@@ -483,16 +496,18 @@ static inline void enic_queue_wq_skb_tso(struct enic *enic,
dma_addr = skb_frag_dma_map(&enic->pdev->dev, frag,
offset, len,
DMA_TO_DEVICE);
- enic_queue_wq_desc_cont(wq, skb,
- dma_addr,
- len,
- (len_left == 0) &&
- (len == frag_len_left), /* EOP? */
- loopback);
+ if (unlikely(enic_dma_map_check(enic, dma_addr)))
+ return -ENOMEM;
+ enic_queue_wq_desc_cont(wq, skb, dma_addr, len,
+ (len_left == 0) &&
+ (len == frag_len_left),/*EOP*/
+ loopback);
frag_len_left -= len;
offset += len;
}
}
+
+ return 0;
}
static inline void enic_queue_wq_skb(struct enic *enic,
@@ -502,6 +517,7 @@ static inline void enic_queue_wq_skb(struct enic *enic,
unsigned int vlan_tag = 0;
int vlan_tag_insert = 0;
int loopback = 0;
+ int err;
if (vlan_tx_tag_present(skb)) {
/* VLAN tag from trunking driver */
@@ -513,14 +529,30 @@ static inline void enic_queue_wq_skb(struct enic *enic,
}
if (mss)
- enic_queue_wq_skb_tso(enic, wq, skb, mss,
- vlan_tag_insert, vlan_tag, loopback);
+ err = enic_queue_wq_skb_tso(enic, wq, skb, mss,
+ vlan_tag_insert, vlan_tag,
+ loopback);
else if (skb->ip_summed == CHECKSUM_PARTIAL)
- enic_queue_wq_skb_csum_l4(enic, wq, skb,
- vlan_tag_insert, vlan_tag, loopback);
+ err = enic_queue_wq_skb_csum_l4(enic, wq, skb, vlan_tag_insert,
+ vlan_tag, loopback);
else
- enic_queue_wq_skb_vlan(enic, wq, skb,
- vlan_tag_insert, vlan_tag, loopback);
+ err = enic_queue_wq_skb_vlan(enic, wq, skb, vlan_tag_insert,
+ vlan_tag, loopback);
+ if (unlikely(err)) {
+ struct vnic_wq_buf *buf;
+
+ buf = wq->to_use->prev;
+ /* while not EOP of previous pkt && queue not empty.
+ * For all non EOP bufs, os_buf is NULL.
+ */
+ while (!buf->os_buf && (buf->next != wq->to_clean)) {
+ enic_free_wq_buf(wq, buf);
+ wq->ring.desc_avail++;
+ buf = buf->prev;
+ }
+ wq->to_use = buf->next;
+ dev_kfree_skb(skb);
+ }
}
/* netif_tx_lock held, process context with BHs disabled, or BH */
@@ -950,8 +982,12 @@ static int enic_rq_alloc_buf(struct vnic_rq *rq)
if (!skb)
return -ENOMEM;
- dma_addr = pci_map_single(enic->pdev, skb->data,
- len, PCI_DMA_FROMDEVICE);
+ dma_addr = pci_map_single(enic->pdev, skb->data, len,
+ PCI_DMA_FROMDEVICE);
+ if (unlikely(enic_dma_map_check(enic, dma_addr))) {
+ dev_kfree_skb(skb);
+ return -ENOMEM;
+ }
enic_queue_rq_desc(rq, skb, os_buf_index,
dma_addr, len);
--
2.2.1
^ permalink raw reply related
* [PATCH net-next 1/3] enic: make vnic_wq_buf doubly linked
From: Govindarajulu Varadarajan @ 2014-12-24 10:29 UTC (permalink / raw)
To: davem, netdev, sassmann; +Cc: ssujith, benve, Govindarajulu Varadarajan
In-Reply-To: <1419416978-1008-1-git-send-email-_govind@gmx.com>
This patch makes vnic_wq_buf doubly liked list. This is needed for dma_mapping
error check, in case some frag's dma map fails, we need to move back and remove
previously queued buffers.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/cisco/enic/vnic_wq.c | 3 +++
drivers/net/ethernet/cisco/enic/vnic_wq.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/cisco/enic/vnic_wq.c b/drivers/net/ethernet/cisco/enic/vnic_wq.c
index 3e6b8d5..b5a1c93 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_wq.c
+++ b/drivers/net/ethernet/cisco/enic/vnic_wq.c
@@ -47,11 +47,14 @@ static int vnic_wq_alloc_bufs(struct vnic_wq *wq)
wq->ring.desc_size * buf->index;
if (buf->index + 1 == count) {
buf->next = wq->bufs[0];
+ buf->next->prev = buf;
break;
} else if (j + 1 == VNIC_WQ_BUF_BLK_ENTRIES(count)) {
buf->next = wq->bufs[i + 1];
+ buf->next->prev = buf;
} else {
buf->next = buf + 1;
+ buf->next->prev = buf;
buf++;
}
}
diff --git a/drivers/net/ethernet/cisco/enic/vnic_wq.h b/drivers/net/ethernet/cisco/enic/vnic_wq.h
index 816f1ad..2961543 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_wq.h
+++ b/drivers/net/ethernet/cisco/enic/vnic_wq.h
@@ -62,6 +62,7 @@ struct vnic_wq_buf {
uint8_t cq_entry; /* Gets completion event from hw */
uint8_t desc_skip_cnt; /* Num descs to occupy */
uint8_t compressed_send; /* Both hdr and payload in one desc */
+ struct vnic_wq_buf *prev;
};
/* Break the vnic_wq_buf allocations into blocks of 32/64 entries */
--
2.2.1
^ permalink raw reply related
* [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support
From: Fugang Duan @ 2014-12-24 9:30 UTC (permalink / raw)
To: shawn.guo, davem; +Cc: netdev, bhutchings, stephen, b38611
The patch series enable FEC Wake-on-LAN feature for i.MX6q/dl and i.MX6SX SOCs.
FEC HW support sleep mode, when system in suspend status with FEC all clock gate
off, magic packet can wake up system. For different SOCs, there have special SOC
GPR register to let FEC enter sleep mode or exit sleep mode, add these to platform
callback for driver' call.
Patch#1: add WOL interface supports.
Patch#2: add SOC special sleep of/off operations for driver's sleep callback.
Patch#3: add magic pattern support for devicetree.
Fugang Duan (3):
net: fec: add Wake-on-LAN support
ARM: imx: add FEC sleep mode callback function
ARM: dts: imx6qdl: enable FEC magic-packet feature
Documentation/devicetree/bindings/net/fsl-fec.txt | 2 +
arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 1 +
arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 1 +
arch/arm/mach-imx/mach-imx6q.c | 41 ++++++++-
arch/arm/mach-imx/mach-imx6sx.c | 50 ++++++++++
drivers/net/ethernet/freescale/fec.h | 2 +
drivers/net/ethernet/freescale/fec_main.c | 104 +++++++++++++++++++--
include/linux/fec.h | 1 +
8 files changed, 191 insertions(+), 11 deletions(-)
--
1.7.8
^ permalink raw reply
* [PATCH net-next v1 3/3] ARM: dts: imx6qdl: enable FEC magic-packet feature
From: Fugang Duan @ 2014-12-24 9:30 UTC (permalink / raw)
To: shawn.guo, davem; +Cc: netdev, bhutchings, stephen, b38611
In-Reply-To: <1419413441-3406-1-git-send-email-b38611@freescale.com>
Add FEC magic-packet feature support.
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 1 +
arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd6..327d362 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -67,6 +67,7 @@
phy-mode = "rgmii";
interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_HIGH>,
<&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
+ fsl,magic-packet;
status = "okay";
};
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index f1cd214..6bfd0bc 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -160,6 +160,7 @@
pinctrl-0 = <&pinctrl_enet>;
phy-mode = "rgmii";
phy-reset-gpios = <&gpio1 25 0>;
+ fsl,magic-packet;
status = "okay";
};
--
1.7.8
^ permalink raw reply related
* [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function
From: Fugang Duan @ 2014-12-24 9:30 UTC (permalink / raw)
To: shawn.guo, davem; +Cc: netdev, bhutchings, stephen, b38611
In-Reply-To: <1419413441-3406-1-git-send-email-b38611@freescale.com>
i.MX6q/dl, i.MX6SX SOCs enet support sleep mode that magic packet can
wake up system in suspend status. For different SOCs, there have some
SOC specifical GPR register to set sleep on/off mode. So add these to
callback function for driver.
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
arch/arm/mach-imx/mach-imx6q.c | 41 +++++++++++++++++++++++++++++++-
arch/arm/mach-imx/mach-imx6sx.c | 50 +++++++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 5057d61..2f76168 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -31,6 +31,8 @@
#include <linux/micrel_phy.h>
#include <linux/mfd/syscon.h>
#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
+#include <linux/fec.h>
+#include <linux/netdevice.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
#include <asm/system_misc.h>
@@ -39,6 +41,35 @@
#include "cpuidle.h"
#include "hardware.h"
+static struct fec_platform_data fec_pdata;
+
+static void imx6q_fec_sleep_enable(int enabled)
+{
+ struct regmap *gpr;
+
+ gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+ if (!IS_ERR(gpr)) {
+ if (enabled)
+ regmap_update_bits(gpr, IOMUXC_GPR13,
+ IMX6Q_GPR13_ENET_STOP_REQ,
+ IMX6Q_GPR13_ENET_STOP_REQ);
+
+ else
+ regmap_update_bits(gpr, IOMUXC_GPR13,
+ IMX6Q_GPR13_ENET_STOP_REQ, 0);
+ } else
+ pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");
+}
+
+static void __init imx6q_enet_plt_init(void)
+{
+ struct device_node *np;
+
+ np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@02188000");
+ if (np && of_get_property(np, "fsl,magic-packet", NULL))
+ fec_pdata.sleep_mode_enable = imx6q_fec_sleep_enable;
+}
+
/* For imx6q sabrelite board: set KSZ9021RN RGMII pad skew */
static int ksz9021rn_phy_fixup(struct phy_device *phydev)
{
@@ -261,6 +292,12 @@ static void __init imx6q_axi_init(void)
}
}
+/* Add auxdata to pass platform data */
+static const struct of_dev_auxdata imx6q_auxdata_lookup[] __initconst = {
+ OF_DEV_AUXDATA("fsl,imx6q-fec", 0x02188000, NULL, &fec_pdata),
+ { /* sentinel */ }
+};
+
static void __init imx6q_init_machine(void)
{
struct device *parent;
@@ -274,11 +311,13 @@ static void __init imx6q_init_machine(void)
imx6q_enet_phy_init();
- of_platform_populate(NULL, of_default_bus_match_table, NULL, parent);
+ of_platform_populate(NULL, of_default_bus_match_table,
+ imx6q_auxdata_lookup, parent);
imx_anatop_init();
cpu_is_imx6q() ? imx6q_pm_init() : imx6dl_pm_init();
imx6q_1588_init();
+ imx6q_enet_plt_init();
imx6q_axi_init();
}
diff --git a/arch/arm/mach-imx/mach-imx6sx.c b/arch/arm/mach-imx/mach-imx6sx.c
index 7a96c65..747b012 100644
--- a/arch/arm/mach-imx/mach-imx6sx.c
+++ b/arch/arm/mach-imx/mach-imx6sx.c
@@ -12,12 +12,62 @@
#include <linux/regmap.h>
#include <linux/mfd/syscon.h>
#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
+#include <linux/fec.h>
+#include <linux/netdevice.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
#include "common.h"
#include "cpuidle.h"
+static struct fec_platform_data fec_pdata[2];
+
+static void imx6sx_fec1_sleep_enable(int enabled)
+{
+ struct regmap *gpr;
+
+ gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sx-iomuxc-gpr");
+ if (!IS_ERR(gpr)) {
+ if (enabled)
+ regmap_update_bits(gpr, IOMUXC_GPR4,
+ IMX6SX_GPR4_FEC_ENET1_STOP_REQ,
+ IMX6SX_GPR4_FEC_ENET1_STOP_REQ);
+ else
+ regmap_update_bits(gpr, IOMUXC_GPR4,
+ IMX6SX_GPR4_FEC_ENET1_STOP_REQ, 0);
+ } else
+ pr_err("failed to find fsl,imx6sx-iomux-gpr regmap\n");
+}
+
+static void imx6sx_fec2_sleep_enable(int enabled)
+{
+ struct regmap *gpr;
+
+ gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sx-iomuxc-gpr");
+ if (!IS_ERR(gpr)) {
+ if (enabled)
+ regmap_update_bits(gpr, IOMUXC_GPR4,
+ IMX6SX_GPR4_FEC_ENET2_STOP_REQ,
+ IMX6SX_GPR4_FEC_ENET2_STOP_REQ);
+ else
+ regmap_update_bits(gpr, IOMUXC_GPR4,
+ IMX6SX_GPR4_FEC_ENET2_STOP_REQ, 0);
+ } else
+ pr_err("failed to find fsl,imx6sx-iomux-gpr regmap\n");
+}
+
+static void __init imx6sx_enet_plt_init(void)
+{
+ struct device_node *np;
+
+ np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@02188000");
+ if (np && of_get_property(np, "fsl,magic-packet", NULL))
+ fec_pdata[0].sleep_mode_enable = imx6sx_fec1_sleep_enable;
+ np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@021b4000");
+ if (np && of_get_property(np, "fsl,magic-packet", NULL))
+ fec_pdata[1].sleep_mode_enable = imx6sx_fec2_sleep_enable;
+}
+
static int ar8031_phy_fixup(struct phy_device *dev)
{
u16 val;
--
1.7.8
^ permalink raw reply related
* [PATCH net-next v1 1/3] net: fec: add Wake-on-LAN support
From: Fugang Duan @ 2014-12-24 9:30 UTC (permalink / raw)
To: shawn.guo, davem; +Cc: netdev, bhutchings, stephen, b38611
In-Reply-To: <1419413441-3406-1-git-send-email-b38611@freescale.com>
Support for Wake-on-LAN using Magic Packet. ENET IP supports sleep mode
in low power status, when system enter suspend status, Magic packet can
wake up system even if all SOC clocks are gate. The patch doing below things:
- flagging the device as a wakeup source for the system, as well as
its Wake-on-LAN interrupt
- prepare the hardware for entering WoL mode
- add standard ethtool WOL interface
- enable the ENET interrupt to wake us
Tested on i.MX6q/dl sabresd, sabreauto boards, i.MX6SX arm2 boards.
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
Documentation/devicetree/bindings/net/fsl-fec.txt | 2 +
drivers/net/ethernet/freescale/fec.h | 2 +
drivers/net/ethernet/freescale/fec_main.c | 104 +++++++++++++++++++--
include/linux/fec.h | 1 +
4 files changed, 99 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 0c8775c..a9eb611 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -22,6 +22,8 @@ Optional properties:
- fsl,num-rx-queues : The property is valid for enet-avb IP, which supports
hw multi queues. Should specify the rx queue number, otherwise set rx queue
number to 1.
+- fsl,magic-packet : If present, indicates that the hardware supports waking
+ up via magic packet.
Optional subnodes:
- mdio : specifies the mdio bus in the FEC, used as a container for phy nodes
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 469691a..e51ab5d 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -356,6 +356,7 @@ struct bufdesc_ex {
#define FEC_ENET_RXB ((uint)0x01000000) /* A buffer was received */
#define FEC_ENET_MII ((uint)0x00800000) /* MII interrupt */
#define FEC_ENET_EBERR ((uint)0x00400000) /* SDMA bus error */
+#define FEC_ENET_WAKEUP ((uint)0x00020000) /* Wakeup request */
#define FEC_ENET_TXF (FEC_ENET_TXF_0 | FEC_ENET_TXF_1 | FEC_ENET_TXF_2)
#define FEC_ENET_RXF (FEC_ENET_RXF_0 | FEC_ENET_RXF_1 | FEC_ENET_RXF_2)
#define FEC_ENET_TS_AVAIL ((uint)0x00010000)
@@ -511,6 +512,7 @@ struct fec_enet_private {
int irq[FEC_IRQ_NUM];
bool bufdesc_ex;
int pause_flag;
+ int wol_flag;
u32 quirks;
struct napi_struct napi;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5c4a8bd..615b51d 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -187,6 +187,9 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
#define FEC_MMFR_RA(v) ((v & 0x1f) << 18)
#define FEC_MMFR_TA (2 << 16)
#define FEC_MMFR_DATA(v) (v & 0xffff)
+/* FEC ECR bits definition */
+#define FEC_ECR_MAGICEN (1 << 2)
+#define FEC_ECR_SLEEP (1 << 3)
#define FEC_MII_TIMEOUT 30000 /* us */
@@ -195,6 +198,9 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
#define FEC_PAUSE_FLAG_AUTONEG 0x1
#define FEC_PAUSE_FLAG_ENABLE 0x2
+#define FEC_WOL_HAS_MAGIC_PACKET (0x1 << 0)
+#define FEC_WOL_FLAG_ENABLE (0x1 << 1)
+#define FEC_WOL_FLAG_SLEEP_ON (0x1 << 2)
#define COPYBREAK_DEFAULT 256
@@ -1089,7 +1095,9 @@ static void
fec_stop(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
+ struct fec_platform_data *pdata = fep->pdev->dev.platform_data;
u32 rmii_mode = readl(fep->hwp + FEC_R_CNTRL) & (1 << 8);
+ u32 val;
/* We cannot expect a graceful transmit stop without link !!! */
if (fep->link) {
@@ -1103,17 +1111,28 @@ fec_stop(struct net_device *ndev)
* For i.MX6SX SOC, enet use AXI bus, we use disable MAC
* instead of reset MAC itself.
*/
- if (fep->quirks & FEC_QUIRK_HAS_AVB) {
- writel(0, fep->hwp + FEC_ECNTRL);
+ if (!(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) {
+ if (fep->quirks & FEC_QUIRK_HAS_AVB) {
+ writel(0, fep->hwp + FEC_ECNTRL);
+ } else {
+ writel(1, fep->hwp + FEC_ECNTRL);
+ udelay(10);
+ }
+ writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
} else {
- writel(1, fep->hwp + FEC_ECNTRL);
- udelay(10);
+ writel(FEC_DEFAULT_IMASK | FEC_ENET_WAKEUP, fep->hwp + FEC_IMASK);
+ val = readl(fep->hwp + FEC_ECNTRL);
+ val |= (FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
+ writel(val, fep->hwp + FEC_ECNTRL);
+
+ if (pdata && pdata->sleep_mode_enable)
+ pdata->sleep_mode_enable(true);
}
writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
- writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
/* We have to keep ENET enabled to have MII interrupt stay working */
- if (fep->quirks & FEC_QUIRK_ENET_MAC) {
+ if (fep->quirks & FEC_QUIRK_ENET_MAC &&
+ !(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) {
writel(2, fep->hwp + FEC_ECNTRL);
writel(rmii_mode, fep->hwp + FEC_R_CNTRL);
}
@@ -2427,6 +2446,44 @@ static int fec_enet_set_tunable(struct net_device *netdev,
return ret;
}
+static void
+fec_enet_get_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
+{
+ struct fec_enet_private *fep = netdev_priv(ndev);
+
+ if (fep->wol_flag & FEC_WOL_HAS_MAGIC_PACKET) {
+ wol->supported = WAKE_MAGIC;
+ wol->wolopts = fep->wol_flag & FEC_WOL_FLAG_ENABLE ? WAKE_MAGIC : 0;
+ } else {
+ wol->supported = wol->wolopts = 0;
+ }
+}
+
+static int
+fec_enet_set_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
+{
+ struct fec_enet_private *fep = netdev_priv(ndev);
+
+ if (!(fep->wol_flag & FEC_WOL_HAS_MAGIC_PACKET))
+ return -EINVAL;
+
+ if (wol->wolopts & ~WAKE_MAGIC)
+ return -EINVAL;
+
+ device_set_wakeup_enable(&ndev->dev, wol->wolopts & WAKE_MAGIC);
+ if (device_may_wakeup(&ndev->dev)) {
+ fep->wol_flag |= FEC_WOL_FLAG_ENABLE;
+ if (fep->irq[0] > 0)
+ enable_irq_wake(fep->irq[0]);
+ } else {
+ fep->wol_flag &= (~FEC_WOL_FLAG_ENABLE);
+ if (fep->irq[0] > 0)
+ disable_irq_wake(fep->irq[0]);
+ }
+
+ return 0;
+}
+
static const struct ethtool_ops fec_enet_ethtool_ops = {
.get_settings = fec_enet_get_settings,
.set_settings = fec_enet_set_settings,
@@ -2445,6 +2502,8 @@ static const struct ethtool_ops fec_enet_ethtool_ops = {
.get_ts_info = fec_enet_get_ts_info,
.get_tunable = fec_enet_get_tunable,
.set_tunable = fec_enet_set_tunable,
+ .get_wol = fec_enet_get_wol,
+ .set_wol = fec_enet_set_wol,
};
static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
@@ -2705,6 +2764,9 @@ fec_enet_open(struct net_device *ndev)
phy_start(fep->phy_dev);
netif_tx_start_all_queues(ndev);
+ device_set_wakeup_enable(&ndev->dev, fep->wol_flag &
+ FEC_WOL_FLAG_ENABLE);
+
return 0;
err_enet_mii_probe:
@@ -3153,6 +3215,9 @@ fec_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, ndev);
+ if (of_get_property(np, "fsl,magic-packet", NULL))
+ fep->wol_flag |= FEC_WOL_HAS_MAGIC_PACKET;
+
phy_node = of_parse_phandle(np, "phy-handle", 0);
if (!phy_node && of_phy_is_fixed_link(np)) {
ret = of_phy_register_fixed_link(np);
@@ -3247,6 +3312,8 @@ fec_probe(struct platform_device *pdev)
0, pdev->name, ndev);
if (ret)
goto failed_irq;
+
+ fep->irq[i] = irq;
}
init_completion(&fep->mdio_done);
@@ -3263,6 +3330,9 @@ fec_probe(struct platform_device *pdev)
if (ret)
goto failed_register;
+ device_init_wakeup(&ndev->dev, fep->wol_flag &
+ FEC_WOL_HAS_MAGIC_PACKET);
+
if (fep->bufdesc_ex && fep->ptp_clock)
netdev_info(ndev, "registered PHC device %d\n", fep->dev_id);
@@ -3316,6 +3386,8 @@ static int __maybe_unused fec_suspend(struct device *dev)
rtnl_lock();
if (netif_running(ndev)) {
+ if (fep->wol_flag & FEC_WOL_FLAG_ENABLE)
+ fep->wol_flag |= FEC_WOL_FLAG_SLEEP_ON;
phy_stop(fep->phy_dev);
napi_disable(&fep->napi);
netif_tx_lock_bh(ndev);
@@ -3323,11 +3395,12 @@ static int __maybe_unused fec_suspend(struct device *dev)
netif_tx_unlock_bh(ndev);
fec_stop(ndev);
fec_enet_clk_enable(ndev, false);
- pinctrl_pm_select_sleep_state(&fep->pdev->dev);
+ if (!(fep->wol_flag & FEC_WOL_FLAG_ENABLE))
+ pinctrl_pm_select_sleep_state(&fep->pdev->dev);
}
rtnl_unlock();
- if (fep->reg_phy)
+ if (fep->reg_phy && !(fep->wol_flag & FEC_WOL_FLAG_ENABLE))
regulator_disable(fep->reg_phy);
/* SOC supply clock to phy, when clock is disabled, phy link down
@@ -3343,9 +3416,11 @@ static int __maybe_unused fec_resume(struct device *dev)
{
struct net_device *ndev = dev_get_drvdata(dev);
struct fec_enet_private *fep = netdev_priv(ndev);
+ struct fec_platform_data *pdata = fep->pdev->dev.platform_data;
int ret;
+ int val;
- if (fep->reg_phy) {
+ if (fep->reg_phy && !(fep->wol_flag & FEC_WOL_FLAG_ENABLE)) {
ret = regulator_enable(fep->reg_phy);
if (ret)
return ret;
@@ -3353,12 +3428,21 @@ static int __maybe_unused fec_resume(struct device *dev)
rtnl_lock();
if (netif_running(ndev)) {
- pinctrl_pm_select_default_state(&fep->pdev->dev);
ret = fec_enet_clk_enable(ndev, true);
if (ret) {
rtnl_unlock();
goto failed_clk;
}
+ if (fep->wol_flag & FEC_WOL_FLAG_ENABLE) {
+ if (pdata && pdata->sleep_mode_enable)
+ pdata->sleep_mode_enable(false);
+ val = readl(fep->hwp + FEC_ECNTRL);
+ val &= ~(FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
+ writel(val, fep->hwp + FEC_ECNTRL);
+ fep->wol_flag &= ~FEC_WOL_FLAG_SLEEP_ON;
+ } else {
+ pinctrl_pm_select_default_state(&fep->pdev->dev);
+ }
fec_restart(ndev);
netif_tx_lock_bh(ndev);
netif_device_attach(ndev);
diff --git a/include/linux/fec.h b/include/linux/fec.h
index bcff455..1454a50 100644
--- a/include/linux/fec.h
+++ b/include/linux/fec.h
@@ -19,6 +19,7 @@
struct fec_platform_data {
phy_interface_t phy;
unsigned char mac[ETH_ALEN];
+ void (*sleep_mode_enable)(int enabled);
};
#endif
--
1.7.8
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox