* [PATCH v4 4/4] can: kvaser_usb: Retry first bulk transfer on -ETIMEDOUT
From: Ahmed S. Darwish @ 2015-01-11 20:45 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: Greg Kroah-Hartman, Linux-USB, Linux-CAN, netdev, LKML
In-Reply-To: <20150111203612.GA8999@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
(This is a draft patch, I'm not sure if this fixes the USB
bug or only its psymptom. Feedback from the linux-usb folks
is really appreciated.)
When plugging the Kvaser USB/CAN dongle the first time, everything
works as expected and all of the transfers from and to the USB
device succeeds.
Meanwhile, after unplugging the device and plugging it again, the
first bulk transfer _always_ returns an -ETIMEDOUT. The following
behaviour was observied:
- Setting higher timeout values for the first bulk transfer never
solved the issue.
- Unloading, then loading, our kvaser_usb module in question
__always__ solved the issue.
- Checking first bulk transfer status, and retry the transfer
again in case of an -ETIMEDOUT also __always__ solved the issue.
This is what the patch below does.
- In the testing done so far, this issue appears only on laptops
but never on PCs (possibly power related?)
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/kvaser_usb.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index da47d17..5925637 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1927,7 +1927,7 @@ static int kvaser_usb_probe(struct usb_interface *intf,
{
struct kvaser_usb *dev;
int err = -ENOMEM;
- int i;
+ int i, retry = 3;
dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL);
if (!dev)
@@ -1956,7 +1956,16 @@ static int kvaser_usb_probe(struct usb_interface *intf,
usb_set_intfdata(intf, dev);
- err = kvaser_usb_get_software_info(dev);
+ /* On some x86 laptops, plugging a USBCAN device again after
+ * an unplug makes the firmware always ignore the very first
+ * command. For such a case, provide some room for retries
+ * instead of completly exiting the driver.
+ */
+ while (retry--) {
+ err = kvaser_usb_get_software_info(dev);
+ if (err != -ETIMEDOUT)
+ break;
+ }
if (err) {
dev_err(&intf->dev,
"Cannot get software infos, error %d\n", err);
--
1.9.1
^ permalink raw reply related
* [PATCH v4 3/4] can: kvaser_usb: Add support for the Usbcan-II family
From: Ahmed S. Darwish @ 2015-01-11 20:36 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20150111201519.GC8855@linux>
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.
List of new devices supported by this driver update:
- Kvaser USBcan II HS/HS
- Kvaser USBcan II HS/LS
- Kvaser USBcan Rugged ("USBcan Rev B")
- Kvaser Memorator HS/HS
- Kvaser Memorator HS/LS
- Scania VCI2 (if you have the Kvaser logo on top)
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/Kconfig | 8 +-
drivers/net/can/usb/kvaser_usb.c | 612 ++++++++++++++++++++++++++++++---------
2 files changed, 487 insertions(+), 133 deletions(-)
** V4 Changelog:
- Use type-safe C methods instead of cpp macros
- Further clarify the code and comments on error events channel arbitration
- Remove defensive checks against non-existing families
- Re-order methods to remove forward declarations
- Smaller stuff spotted by earlier review (function prefexes, etc.)
** V3 Changelog:
- Fix padding for the usbcan_msg_tx_acknowledge command
- Remove kvaser_usb->max_channels and the MAX_NET_DEVICES macro
- Rename commands to CMD_LEAF_xxx and CMD_USBCAN_xxx
- Apply checkpatch.pl suggestions ('net/' comments, multi-line strings, etc.)
** V2 Changelog:
- Update Kconfig entries
- Use actual number of CAN channels (instead of max) where appropriate
- Rebase over a new set of UsbcanII-independent driver fixes
diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
index a77db919..f6f5500 100644
--- a/drivers/net/can/usb/Kconfig
+++ b/drivers/net/can/usb/Kconfig
@@ -25,7 +25,7 @@ config CAN_KVASER_USB
tristate "Kvaser CAN/USB interface"
---help---
This driver adds support for Kvaser CAN/USB devices like Kvaser
- Leaf Light.
+ Leaf Light and Kvaser USBcan II.
The driver provides support for the following devices:
- Kvaser Leaf Light
@@ -46,6 +46,12 @@ config CAN_KVASER_USB
- Kvaser USBcan R
- Kvaser Leaf Light v2
- Kvaser Mini PCI Express HS
+ - Kvaser USBcan II HS/HS
+ - Kvaser USBcan II HS/LS
+ - Kvaser USBcan Rugged ("USBcan Rev B")
+ - Kvaser Memorator HS/HS
+ - Kvaser Memorator HS/LS
+ - Scania VCI2 (if you have the Kvaser logo on top)
If unsure, say N.
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 0eb870b..da47d17 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -6,10 +6,12 @@
* 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) 2015 Valeo Corporation
*/
#include <linux/completion.h>
@@ -21,6 +23,15 @@
#include <linux/can/dev.h>
#include <linux/can/error.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 */
@@ -30,8 +41,9 @@
#define RX_BUFFER_SIZE 3072
#define CAN_USB_CLOCK 8000000
#define MAX_NET_DEVICES 3
+#define MAX_USBCAN_NET_DEVICES 2
-/* 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
@@ -56,6 +68,24 @@
#define USB_LEAF_LITE_V2_PRODUCT_ID 288
#define USB_MINI_PCIE_HS_PRODUCT_ID 289
+static inline bool kvaser_is_leaf(const struct usb_device_id *id)
+{
+ return id->idProduct >= USB_LEAF_DEVEL_PRODUCT_ID &&
+ id->idProduct <= 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
+
+static inline bool kvaser_is_usbcan(const struct usb_device_id *id)
+{
+ return id->idProduct >= USB_USBCAN_REVB_PRODUCT_ID &&
+ id->idProduct <= USB_MEMORATOR_PRODUCT_ID;
+}
+
/* USB devices features */
#define KVASER_HAS_SILENT_MODE BIT(0)
#define KVASER_HAS_TXRX_ERRORS BIT(1)
@@ -73,7 +103,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 +128,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 CMD_LEAF_GET_CARD_INFO2 32
+#define CMD_USBCAN_RESET_CLOCK 32
+#define CMD_USBCAN_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 +144,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 CMD_LEAF_USB_THROTTLE 77
+#define CMD_LEAF_LOG_MESSAGE 106
/* error factors */
#define M16C_EF_ACKE BIT(0)
@@ -121,6 +158,14 @@
#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 USBCANII
+ */
+#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 +182,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 +193,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 +258,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 +349,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 +375,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 +392,57 @@ 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;
+/* Summary of a kvaser error event, for a unified Leaf/Usbcan error
+ * handling. Some discrepancies between the two families exist:
+ *
+ * - USBCAN firmware does not report M16C "error factors"
+ * - USBCAN controllers has difficulties reporting if the raised error
+ * event is for ch0 or ch1. They leave such arbitration to the OS
+ * driver by letting it compare error counters with previous values
+ * and decide the error event's channel. Thus for USBCAN, the channel
+ * field is only advisory.
+ */
+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 +458,7 @@ struct kvaser_usb {
u32 fw_version;
unsigned int nchannels;
+ enum kvaser_usb_family family;
bool rxinitdone;
void *rxbuf[MAX_RX_URBS];
@@ -311,6 +482,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 +532,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 +646,14 @@ 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;
+ }
return 0;
}
@@ -482,7 +672,9 @@ 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 > MAX_NET_DEVICES) ||
+ (dev->family == KVASER_USBCAN &&
+ dev->nchannels > MAX_USBCAN_NET_DEVICES))
return -EINVAL;
return 0;
@@ -496,8 +688,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,
@@ -616,53 +810,24 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
}
static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
- const struct kvaser_msg *msg)
+ 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;
-
- 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;
- 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];
- 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;
- break;
- default:
- dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
- msg->id);
- return;
- }
- if (channel >= dev->nchannels) {
+ if (es->channel >= dev->nchannels) {
dev_err(dev->udev->dev.parent,
- "Invalid channel number (%d)\n", channel);
+ "Invalid channel number (%d)\n", es->channel);
return;
}
- priv = dev->nets[channel];
+ 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 +840,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 +852,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 +868,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 +886,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 +902,48 @@ 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;
}
- 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;
@@ -775,6 +952,124 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
netif_rx(skb);
}
+/* For USBCAN, report error to userspace iff the channels's errors counter
+ * has increased, or we're the only channel seeing a bus error state.
+ */
+static void kvaser_usbcan_conditionally_rx_error(const struct kvaser_usb *dev,
+ struct kvaser_error_summary *es)
+{
+ struct kvaser_usb_net_priv *priv;
+ int channel;
+ bool 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];
+ report_error = false;
+
+ if (es->txerr > priv->bec.txerr) {
+ es->usbcan.error_state |= USBCAN_ERROR_STATE_TX_ERROR;
+ report_error = true;
+ }
+ if (es->rxerr > priv->bec.rxerr) {
+ es->usbcan.error_state |= USBCAN_ERROR_STATE_RX_ERROR;
+ report_error = true;
+ }
+ 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 = true;
+ }
+
+ if (report_error)
+ kvaser_usb_rx_error(dev, es);
+}
+
+static void kvaser_usbcan_rx_error(const struct kvaser_usb *dev,
+ const struct kvaser_msg *msg)
+{
+ struct kvaser_error_summary es = { };
+
+ 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;
+ kvaser_usbcan_conditionally_rx_error(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;
+ kvaser_usbcan_conditionally_rx_error(dev, &es);
+
+ /* The USBCAN firmware does not support more than 2 channels.
+ * Now that ch0 was checked, check if ch1 has any errors.
+ */
+ if (dev->nchannels == MAX_USBCAN_NET_DEVICES) {
+ 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;
+ kvaser_usbcan_conditionally_rx_error(dev, &es);
+ }
+ break;
+
+ default:
+ dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
+ msg->id);
+ }
+}
+
+static void kvaser_leaf_rx_error(const struct kvaser_usb *dev,
+ const struct kvaser_msg *msg)
+{
+ struct kvaser_error_summary es = { };
+
+ switch (msg->id) {
+ case CMD_CAN_ERROR_EVENT:
+ 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_LEAF_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:
+ 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",
+ msg->id);
+ return;
+ }
+
+ kvaser_usb_rx_error(dev, &es);
+}
+
static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
const struct kvaser_msg *msg)
{
@@ -782,16 +1077,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) {
stats->rx_over_errors++;
stats->rx_errors++;
@@ -817,7 +1112,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,
@@ -828,19 +1124,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)) {
- kvaser_usb_rx_error(dev, msg);
+ if ((msg->u.rx_can_header.flag & MSG_FLAG_ERROR_FRAME) &&
+ (dev->family == KVASER_LEAF && msg->id == CMD_LEAF_LOG_MESSAGE)) {
+ kvaser_leaf_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;
}
@@ -850,38 +1159,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 == CMD_LEAF_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);
}
@@ -944,21 +1252,35 @@ 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 CMD_LEAF_LOG_MESSAGE:
+ if (dev->family != KVASER_LEAF)
+ goto warn;
kvaser_usb_rx_can_msg(dev, msg);
break;
case CMD_CHIP_STATE_EVENT:
case CMD_CAN_ERROR_EVENT:
- kvaser_usb_rx_error(dev, msg);
+ if (dev->family == KVASER_LEAF)
+ kvaser_leaf_rx_error(dev, msg);
+ else
+ kvaser_usbcan_rx_error(dev, msg);
break;
case CMD_TX_ACKNOWLEDGE:
kvaser_usb_tx_acknowledge(dev, msg);
break;
+ /* Ignored messages */
+ case CMD_USBCAN_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;
}
@@ -1178,7 +1500,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->nchannels; i++) {
struct kvaser_usb_net_priv *priv = dev->nets[i];
if (priv)
@@ -1286,6 +1608,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;
if (can_dropped_invalid_skb(netdev, skb))
return NETDEV_TX_OK;
@@ -1307,9 +1630,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;
@@ -1327,7 +1664,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 +1933,17 @@ static int kvaser_usb_probe(struct usb_interface *intf,
if (!dev)
return -ENOMEM;
+ if (kvaser_is_leaf(id)) {
+ dev->family = KVASER_LEAF;
+ } else if (kvaser_is_usbcan(id)) {
+ dev->family = KVASER_USBCAN;
+ } else {
+ dev_err(&intf->dev,
+ "Product ID (%d) does not belong to any known Kvaser USB family",
+ id->idProduct);
+ return -ENODEV;
+ }
+
err = kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
if (err) {
dev_err(&intf->dev, "Cannot get usb endpoint(s)");
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next RFC 5/5] net-timestamp: tx timestamping default mode flag
From: Richard Cochran @ 2015-01-11 20:32 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem, eric.dumazet, luto
In-Reply-To: <1420824719-28848-6-git-send-email-willemb@google.com>
On Fri, Jan 09, 2015 at 12:31:59PM -0500, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> The number of timestamping points along the transmit path has grown,
> as have the options. Preferred behavior is to request timestamps with
> ID, without data (which enables batching) and for all supported
> timestamp points. Define a short option that enables all these
> defaults.
This "preferred behavior" is subjective, and it depends on the
application. I am sure it reflects your own interest, but for people
doing PTP over UDP or raw, it is a bit misleading.
I would drop this default and just let applications define their own.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH RESEND 2/2] wlcore: align member-assigns in a structure-copy block
From: Giel van Schijndel @ 2015-01-11 20:32 UTC (permalink / raw)
To: Eliad Peller
Cc: Kalle Valo, LKML, John W. Linville, Arik Nemtsov,
open list:TI WILINK WIRELES..., open list:NETWORKING DRIVERS
In-Reply-To: <CAB3XZEcm4+=dYC-8_m_7YRdET_wPkWUYHcDuy9mCKdxkCFEnQA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3868 bytes --]
On Sun, Jan 11, 2015 at 12:22:49 +0200, Eliad Peller wrote:
> On Fri, Jan 9, 2015 at 7:03 PM, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Giel van Schijndel <me@mortis.eu> writes:
>>> This highlights the differences (e.g. the bug fixed in the previous
>>> commit).
>>>
>>> Signed-off-by: Giel van Schijndel <me@mortis.eu>
>>> ---
>>> drivers/net/wireless/ti/wlcore/acx.c | 22 +++++++++++-----------
>>> 1 file changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c
>>> index f28fa3b..93a2fa8 100644
>>> --- a/drivers/net/wireless/ti/wlcore/acx.c
>>> +++ b/drivers/net/wireless/ti/wlcore/acx.c
>>> @@ -1715,17 +1715,17 @@ int wl12xx_acx_config_hangover(struct wl1271 *wl)
>>> goto out;
>>> }
>>>
>>> - acx->recover_time = cpu_to_le32(conf->recover_time);
>>> - acx->hangover_period = conf->hangover_period;
>>> - acx->dynamic_mode = conf->dynamic_mode;
>>> - acx->early_termination_mode = conf->early_termination_mode;
>>> - acx->max_period = conf->max_period;
>>> - acx->min_period = conf->min_period;
>>> - acx->increase_delta = conf->increase_delta;
>>> - acx->decrease_delta = conf->decrease_delta;
>>> - acx->quiet_time = conf->quiet_time;
>>> - acx->increase_time = conf->increase_time;
>>> - acx->window_size = conf->window_size;
>>> + acx->recover_time = cpu_to_le32(conf->recover_time);
>>> + acx->hangover_period = conf->hangover_period;
>>> + acx->dynamic_mode = conf->dynamic_mode;
>>> + acx->early_termination_mode = conf->early_termination_mode;
>>> + acx->max_period = conf->max_period;
>>> + acx->min_period = conf->min_period;
>>> + acx->increase_delta = conf->increase_delta;
>>> + acx->decrease_delta = conf->decrease_delta;
>>> + acx->quiet_time = conf->quiet_time;
>>> + acx->increase_time = conf->increase_time;
>>> + acx->window_size = conf->window_size;
>>
>> I would like to get an ACK from one of the wlcore developers if I should
>> apply this (or not).
>>
> I don't have a strong opinion here.
> However, it looks pretty much redundant to take a random blob (which
> was just fixed by a correct patch) and re-indent it.
> The rest of the file doesn't follow this style, so i don't see a good
> reason to apply it here.
>
> I agree such indentation have some benefit, but it won't help with the
> more common use case (of copy-paste error) of copying the wrong field
> (i.e. D->a = S->b instead of D->a = S->a).
> For these cases the macros suggested by Arend and Johannes will do the
> trick. However i usually dislike such macros, as they tend to break
> some IDE features (e.g. auto completion).
> Maybe we can come up with some nice spatch to catch these cases.
What I dislike about those macros is just that they're not as familiar
to any C programmer as the assignment operator, so they make the code
less readable (even if just a little bit).
As for the IDE thing: I try not to use them, but have been told (by
colleagues) that Eclipse is reasonably smart about macros in C. I use
VIM with the clang_complete plugin and that does do proper completion
with expressions containing macros, but not inside macros based on what
the macro expansion would be, like the one above.
That's why I believe this kind of alignment is at least *an* improvement
even if it doesn't solve all possible problems.
--
Met vriendelijke groet,
With kind regards,
Giel van Schijndel
--
"Question: what do you call your programming methodology?
Answer: Faith based development. You code and then pray that it works."
-- John Spelner
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH net-next RFC 1/5] net-timestamp: no-payload option
From: Richard Cochran @ 2015-01-11 20:26 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem, eric.dumazet, luto
In-Reply-To: <1420824719-28848-2-git-send-email-willemb@google.com>
On Fri, Jan 09, 2015 at 12:31:55PM -0500, Willem de Bruijn wrote:
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index a317797..d81ef70 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -440,7 +440,7 @@ static bool ipv4_pktinfo_prepare_errqueue(const struct sock *sk,
>
> if ((ee_origin != SO_EE_ORIGIN_TIMESTAMPING) ||
> (!(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_CMSG)) ||
> - (!skb->dev))
> + (!skb->dev) || (!skb->len))
> return false;
Nit: You have already tested for the condition (!skb->len) ...
>
> info->ipi_spec_dst.s_addr = ip_hdr(skb)->saddr;
> @@ -483,7 +483,7 @@ int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
>
> serr = SKB_EXT_ERR(skb);
>
> - if (sin) {
> + if (sin && skb->len) {
> sin->sin_family = AF_INET;
> sin->sin_addr.s_addr = *(__be32 *)(skb_network_header(skb) +
> serr->addr_offset);
> @@ -496,8 +496,9 @@ int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
> sin = &errhdr.offender;
> sin->sin_family = AF_UNSPEC;
>
> - if (serr->ee.ee_origin == SO_EE_ORIGIN_ICMP ||
> - ipv4_pktinfo_prepare_errqueue(sk, skb, serr->ee.ee_origin)) {
> + if (skb->len &&
> + (serr->ee.ee_origin == SO_EE_ORIGIN_ICMP ||
> + ipv4_pktinfo_prepare_errqueue(sk, skb, serr->ee.ee_origin))) {
... here.
> struct inet_sock *inet = inet_sk(sk);
>
> sin->sin_family = AF_INET;
Thanks,
Richard
^ permalink raw reply
* [PATCH v4 2/4] can: kvaser_usb: Update error counters before exiting on OOM
From: Ahmed S. Darwish @ 2015-01-11 20:15 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20150111201116.GB8855@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Let the error counters be more accurate in case of Out of
Memory conditions.
Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/kvaser_usb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index c32cd61..0eb870b 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -792,6 +792,9 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
}
if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
+ stats->rx_over_errors++;
+ stats->rx_errors++;
+
skb = alloc_can_err_skb(priv->netdev, &cf);
if (!skb) {
stats->rx_dropped++;
@@ -801,9 +804,6 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
cf->can_id |= CAN_ERR_CRTL;
cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
- stats->rx_over_errors++;
- stats->rx_errors++;
-
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
netif_rx(skb);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v4 01/04] can: kvaser_usb: Don't dereference skb after a netif_rx() devices
From: Ahmed S. Darwish @ 2015-01-11 20:11 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20150111200544.GA8855@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
We should not touch the packet after a netif_rx: it might
get freed behind our back.
Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/kvaser_usb.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index cc7bfc0..c32cd61 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -520,10 +520,10 @@ static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
skb = alloc_can_err_skb(priv->netdev, &cf);
if (skb) {
cf->can_id |= CAN_ERR_RESTARTED;
- netif_rx(skb);
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
+ netif_rx(skb);
} else {
netdev_err(priv->netdev,
"No memory left for err_skb\n");
@@ -770,10 +770,9 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
priv->can.state = new_state;
- netif_rx(skb);
-
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
+ netif_rx(skb);
}
static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
@@ -805,10 +804,9 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
stats->rx_over_errors++;
stats->rx_errors++;
- netif_rx(skb);
-
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
+ netif_rx(skb);
}
}
@@ -887,10 +885,9 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
cf->can_dlc);
}
- netif_rx(skb);
-
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
+ netif_rx(skb);
}
static void kvaser_usb_start_chip_reply(const struct kvaser_usb *dev,
--
1.9.1
^ permalink raw reply related
* [PATCH v4 00/04] can: Introduce support for Kvaser USBCAN-II devices
From: Ahmed S. Darwish @ 2015-01-11 20:05 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20141223154654.GB6460@vivalin-002>
Hi,
Now since earlier v3 submission patches #1-3 got merged, this
is a new patch series expanding on patch v3 #4: support for
the USBCAN-II family.
A new series is introduced due to the extra additions suggested
by code review, which required being added in their own
self-contained patches.
Thanks,
Darwish
^ permalink raw reply
* [PATCH] checkpatch: Add likely/unlikely comparison misuse test
From: Joe Perches @ 2015-01-11 19:49 UTC (permalink / raw)
To: Christoph Jaeger, Andrew Morton, Julia Lawall
Cc: Alan, davem, willemb, edumazet, dborkman, netdev, linux-kernel
In-Reply-To: <20150111193404.GN1513@betelgeuse.hsd1.ma.comcast.net>
Add a test for probably likely/unlikely misuses where
the comparison is likely misplaced
if (likely(foo) > 0)
vs
if (likely(foo > 0))
Signed-off-by: Joe Perches <joe@perches.com>
---
On Sun, 2015-01-11 at 14:34 -0500, Christoph Jaeger wrote:
> > drivers/platform/goldfish/goldfish_pipe.c:285: if (unlikely(bufflen) == 0)
>
> Well, the conditional statement works as intended. Of course, the branch
> prediction doesn't.
>
> Coccinelle should be able to check for this kind of likely()/unlikely() usage,
> shouldn't it?
Most likely, checkpatch could too, but not as well.
This misuse isn't very common. (2 in current source?)
scripts/checkpatch.pl | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6afc24b..b8d47dc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5219,6 +5219,13 @@ sub process {
"#define of '$1' is wrong - use Kconfig variables or standard guards instead\n" . $herecurr);
}
+# likely/unlikely comparisons similar to "(likely(foo) > 0)"
+ if ($^V && $^V ge 5.10.0 &&
+ $line =~ /\b((?:un)?likely)\s*\(\s*$FuncArg\s*\)\s*$Compare/) {
+ WARN("LIKELY_MISUSE",
+ "Using $1 should generally have parentheses around the comparison\n" . $herecurr);
+ }
+
# whine mightly about in_atomic
if ($line =~ /\bin_atomic\s*\(/) {
if ($realfile =~ m@^drivers/@) {
^ permalink raw reply related
* [PATCH net-next v1] net: bnx2x: avoid macro redefinition
From: David Decotigny @ 2015-01-11 19:42 UTC (permalink / raw)
To: Ariel Elior, netdev, linux-kernel; +Cc: David Decotigny
From: David Decotigny <decot@googlers.com>
Signed-off-by: David Decotigny <decot@googlers.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index 792ba72..756053c 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -1138,12 +1138,8 @@ struct bnx2x_port {
u32 link_config[LINK_CONFIG_SIZE];
u32 supported[LINK_CONFIG_SIZE];
-/* link settings - missing defines */
-#define SUPPORTED_2500baseX_Full (1 << 15)
u32 advertising[LINK_CONFIG_SIZE];
-/* link settings - missing defines */
-#define ADVERTISED_2500baseX_Full (1 << 15)
u32 phy_addr;
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* Re: [PATCH net] packet: bail out of packet_snd() if L2 header creation fails
From: Christoph Jaeger @ 2015-01-11 19:34 UTC (permalink / raw)
To: Joe Perches
Cc: Alan, davem, willemb, edumazet, dborkman, netdev, linux-kernel
In-Reply-To: <1421002345.9233.1.camel@perches.com>
On Sun, Jan 11, 2015 at 10:52:25AM -0800, Joe Perches wrote:
> On Sun, 2015-01-11 at 13:01 -0500, Christoph Jaeger wrote:
> > Due to a misplaced parenthesis, the expression
> >
> > (unlikely(offset) < 0),
> >
> > which expands to
> >
> > (__builtin_expect(!!(offset), 0) < 0),
>
> Here's another one:
>
> drivers/platform/goldfish/goldfish_pipe.c:285: if (unlikely(bufflen) == 0)
Well, the conditional statement works as intended. Of course, the branch
prediction doesn't.
Coccinelle should be able to check for this kind of likely()/unlikely() usage,
shouldn't it?
~Christoph
^ permalink raw reply
* [PATCH] net: dnet: fix dnet_poll()
From: Eric Dumazet @ 2015-01-11 19:02 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
A NAPI poll() handler is supposed to return exactly the budget when/if
napi_complete() has not been called.
It is also supposed to return number of frames that were received, so
that netdev_budget can have a meaning.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Note : Untested patch, but this driver seems pretty buggy, not sure if
anyone uses it.
drivers/net/ethernet/dnet.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/dnet.c b/drivers/net/ethernet/dnet.c
index a379c3e4b57f..13d00a38a5bd 100644
--- a/drivers/net/ethernet/dnet.c
+++ b/drivers/net/ethernet/dnet.c
@@ -398,13 +398,8 @@ static int dnet_poll(struct napi_struct *napi, int budget)
* break out of while loop if there are no more
* packets waiting
*/
- if (!(dnet_readl(bp, RX_FIFO_WCNT) >> 16)) {
- napi_complete(napi);
- int_enable = dnet_readl(bp, INTR_ENB);
- int_enable |= DNET_INTR_SRC_RX_CMDFIFOAF;
- dnet_writel(bp, int_enable, INTR_ENB);
- return 0;
- }
+ if (!(dnet_readl(bp, RX_FIFO_WCNT) >> 16))
+ break;
cmd_word = dnet_readl(bp, RX_LEN_FIFO);
pkt_len = cmd_word & 0xFFFF;
@@ -433,20 +428,17 @@ static int dnet_poll(struct napi_struct *napi, int budget)
"size %u.\n", dev->name, pkt_len);
}
- budget -= npackets;
-
if (npackets < budget) {
/* We processed all packets available. Tell NAPI it can
- * stop polling then re-enable rx interrupts */
+ * stop polling then re-enable rx interrupts.
+ */
napi_complete(napi);
int_enable = dnet_readl(bp, INTR_ENB);
int_enable |= DNET_INTR_SRC_RX_CMDFIFOAF;
dnet_writel(bp, int_enable, INTR_ENB);
- return 0;
}
- /* There are still packets waiting */
- return 1;
+ return npackets;
}
static irqreturn_t dnet_interrupt(int irq, void *dev_id)
^ permalink raw reply related
* Re: [PATCH net] packet: bail out of packet_snd() if L2 header creation fails
From: Joe Perches @ 2015-01-11 18:52 UTC (permalink / raw)
To: Christoph Jaeger, Alan
Cc: davem, willemb, edumazet, dborkman, netdev, linux-kernel
In-Reply-To: <1420999276-28225-1-git-send-email-cj@linux.com>
On Sun, 2015-01-11 at 13:01 -0500, Christoph Jaeger wrote:
> Due to a misplaced parenthesis, the expression
>
> (unlikely(offset) < 0),
>
> which expands to
>
> (__builtin_expect(!!(offset), 0) < 0),
Here's another one:
drivers/platform/goldfish/goldfish_pipe.c:285: if (unlikely(bufflen) == 0)
^ permalink raw reply
* Re: [PATCH net] packet: bail out of packet_snd() if L2 header creation fails
From: Willem de Bruijn @ 2015-01-11 18:49 UTC (permalink / raw)
To: Eric Dumazet
Cc: Christoph Jaeger, David Miller, Eric Dumazet, Daniel Borkmann,
Network Development, linux-kernel
In-Reply-To: <1421001331.5947.101.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, Jan 11, 2015 at 1:35 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sun, 2015-01-11 at 13:01 -0500, Christoph Jaeger wrote:
>> Due to a misplaced parenthesis, the expression
>>
>> (unlikely(offset) < 0),
>>
>> which expands to
>>
>> (__builtin_expect(!!(offset), 0) < 0),
>>
>> never evaluates to true. Therefore, when sending packets with
>> PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended
>> if the creation of the layer 2 header fails.
>>
>> Spotted by Coverity - CID 1259975 ("Operands don't affect result").
>>
>> Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header")
>> Signed-off-by: Christoph Jaeger <cj@linux.com>
>> ---
>
> Nice catch !
>
> Acked-by: Eric Dumazet <edumazet@google.com>
>
Indeed. I'm responsible for that typo. Thanks a lot for catching it!
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH net] packet: bail out of packet_snd() if L2 header creation fails
From: Eric Dumazet @ 2015-01-11 18:35 UTC (permalink / raw)
To: Christoph Jaeger; +Cc: davem, willemb, edumazet, dborkman, netdev, linux-kernel
In-Reply-To: <1420999276-28225-1-git-send-email-cj@linux.com>
On Sun, 2015-01-11 at 13:01 -0500, Christoph Jaeger wrote:
> Due to a misplaced parenthesis, the expression
>
> (unlikely(offset) < 0),
>
> which expands to
>
> (__builtin_expect(!!(offset), 0) < 0),
>
> never evaluates to true. Therefore, when sending packets with
> PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended
> if the creation of the layer 2 header fails.
>
> Spotted by Coverity - CID 1259975 ("Operands don't affect result").
>
> Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header")
> Signed-off-by: Christoph Jaeger <cj@linux.com>
> ---
Nice catch !
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH net] alx: fix alx_poll()
From: Eric Dumazet @ 2015-01-11 18:32 UTC (permalink / raw)
To: Oded Gabbay
Cc: Johannes Berg, David S. Miller, Eric Dumazet,
netdev@vger.kernel.org, Willem de Bruijn, Bridgman, John,
Elifaz, Dana
In-Reply-To: <1420926614.5947.89.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
Commit d75b1ade567f ("net: less interrupt masking in NAPI") uncovered
wrong alx_poll() behavior.
A NAPI poll() handler is supposed to return exactly the budget when/if
napi_complete() has not been called.
It is also supposed to return number of frames that were received, so
that netdev_budget can have a meaning.
Also, in case of TX pressure, we still have to dequeue received
packets : alx_clean_rx_irq() has to be called even if
alx_clean_tx_irq(alx) returns false, otherwise device is half duplex.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Fixes: d75b1ade567f ("net: less interrupt masking in NAPI")
Reported-by: Oded Gabbay <oded.gabbay@amd.com>
Bisected-by: Oded Gabbay <oded.gabbay@amd.com>
Tested-by: Oded Gabbay <oded.gabbay@amd.com>
---
drivers/net/ethernet/atheros/alx/main.c | 24 +++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index e398eda07298..c8af3ce3ea38 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -184,15 +184,16 @@ static void alx_schedule_reset(struct alx_priv *alx)
schedule_work(&alx->reset_wk);
}
-static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
+static int alx_clean_rx_irq(struct alx_priv *alx, int budget)
{
struct alx_rx_queue *rxq = &alx->rxq;
struct alx_rrd *rrd;
struct alx_buffer *rxb;
struct sk_buff *skb;
u16 length, rfd_cleaned = 0;
+ int work = 0;
- while (budget > 0) {
+ while (work < budget) {
rrd = &rxq->rrd[rxq->rrd_read_idx];
if (!(rrd->word3 & cpu_to_le32(1 << RRD_UPDATED_SHIFT)))
break;
@@ -203,7 +204,7 @@ static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
ALX_GET_FIELD(le32_to_cpu(rrd->word0),
RRD_NOR) != 1) {
alx_schedule_reset(alx);
- return 0;
+ return work;
}
rxb = &rxq->bufs[rxq->read_idx];
@@ -243,7 +244,7 @@ static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
}
napi_gro_receive(&alx->napi, skb);
- budget--;
+ work++;
next_pkt:
if (++rxq->read_idx == alx->rx_ringsz)
@@ -258,21 +259,22 @@ next_pkt:
if (rfd_cleaned)
alx_refill_rx_ring(alx, GFP_ATOMIC);
- return budget > 0;
+ return work;
}
static int alx_poll(struct napi_struct *napi, int budget)
{
struct alx_priv *alx = container_of(napi, struct alx_priv, napi);
struct alx_hw *hw = &alx->hw;
- bool complete = true;
unsigned long flags;
+ bool tx_complete;
+ int work;
- complete = alx_clean_tx_irq(alx) &&
- alx_clean_rx_irq(alx, budget);
+ tx_complete = alx_clean_tx_irq(alx);
+ work = alx_clean_rx_irq(alx, budget);
- if (!complete)
- return 1;
+ if (!tx_complete || work == budget)
+ return budget;
napi_complete(&alx->napi);
@@ -284,7 +286,7 @@ static int alx_poll(struct napi_struct *napi, int budget)
alx_post_write(hw);
- return 0;
+ return work;
}
static irqreturn_t alx_intr_handle(struct alx_priv *alx, u32 intr)
^ permalink raw reply related
* [PATCH net] packet: bail out of packet_snd() if L2 header creation fails
From: Christoph Jaeger @ 2015-01-11 18:01 UTC (permalink / raw)
To: davem; +Cc: willemb, edumazet, dborkman, netdev, linux-kernel,
Christoph Jaeger
Due to a misplaced parenthesis, the expression
(unlikely(offset) < 0),
which expands to
(__builtin_expect(!!(offset), 0) < 0),
never evaluates to true. Therefore, when sending packets with
PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended
if the creation of the layer 2 header fails.
Spotted by Coverity - CID 1259975 ("Operands don't affect result").
Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header")
Signed-off-by: Christoph Jaeger <cj@linux.com>
---
net/packet/af_packet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 6880f34..9cfe2e1 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2517,7 +2517,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
err = -EINVAL;
if (sock->type == SOCK_DGRAM) {
offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len);
- if (unlikely(offset) < 0)
+ if (unlikely(offset < 0))
goto out_free;
} else {
if (ll_header_truncated(dev, len))
--
2.1.0
^ permalink raw reply related
* Re: [PATCH v4] can: Convert to runtime_pm
From: Marc Kleine-Budde @ 2015-01-11 15:41 UTC (permalink / raw)
To: Appana Durga Kedareswara Rao
Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Soren Brinkmann,
grant.likely@linaro.org, wg@grandegger.com
In-Reply-To: <12e7202e137e4d5680185748ebf0cf2d@BY2FFO11FD060.protection.gbl>
[-- Attachment #1: Type: text/plain, Size: 5561 bytes --]
On 01/11/2015 06:34 AM, Appana Durga Kedareswara Rao wrote:
[...]
>>> return ret;
>>> }
>>>
>>> priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
>>> priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
>>>
>>> if (netif_running(ndev)) {
>>> priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>
>> What happens if the device was not in ACTIVE state prior to the
>> runtime_suspend?
>>
>
> I am not sure about the state of CAN at this point of time.
> I just followed what other drivers are following for run time suspend :).
Please check the state of the hardware if you go with bus off into
suspend and then resume.
>>> netif_device_attach(ndev);
>>> netif_start_queue(ndev);
>>> }
>>>
>>> return 0;
>>> }
>>
>>
>>>
>>> @@ -1020,9 +1035,9 @@ static int __maybe_unused xcan_resume(struct
>>> device *dev)
>>>
>>> priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
>>> priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
>>> - priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>>
>>> if (netif_running(ndev)) {
>>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>> netif_device_attach(ndev);
>>> netif_start_queue(ndev);
>>> }
>>> @@ -1030,7 +1045,10 @@ static int __maybe_unused xcan_resume(struct
>> device *dev)
>>> return 0;
>>> }
>>>
>>> -static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend,
>> xcan_resume);
>>> +static const struct dev_pm_ops xcan_dev_pm_ops = {
>>> + SET_SYSTEM_SLEEP_PM_OPS(xcan_suspend, xcan_resume)
>>> + SET_PM_RUNTIME_PM_OPS(xcan_runtime_suspend,
>> xcan_runtime_resume,
>>> +NULL) };
>>>
>>> /**
>>> * xcan_probe - Platform registration call @@ -1071,7 +1089,7 @@
>>> static int xcan_probe(struct platform_device *pdev)
>>> return -ENOMEM;
>>>
>>> priv = netdev_priv(ndev);
>>> - priv->dev = ndev;
>>> + priv->dev = &pdev->dev;
>>> priv->can.bittiming_const = &xcan_bittiming_const;
>>> priv->can.do_set_mode = xcan_do_set_mode;
>>> priv->can.do_get_berr_counter = xcan_get_berr_counter; @@ -
>> 1137,15
>>> +1155,22 @@ static int xcan_probe(struct platform_device *pdev)
>>>
>>> netif_napi_add(ndev, &priv->napi, xcan_rx_poll, rx_max);
>>>
>>> + pm_runtime_set_active(&pdev->dev);
>>> + pm_runtime_irq_safe(&pdev->dev);
>>> + pm_runtime_enable(&pdev->dev);
>>> + pm_runtime_get_sync(&pdev->dev);
>> Check error values?
>
> Ok
>>> +
>>> ret = register_candev(ndev);
>>> if (ret) {
>>> dev_err(&pdev->dev, "fail to register failed (err=%d)\n",
>> ret);
>>> + pm_runtime_put(priv->dev);
>>
>> Please move the pm_runtime_put into the common error exit path.
>>
>
> Ok
>
>>> goto err_unprepare_disable_busclk;
>>> }
>>>
>>> devm_can_led_init(ndev);
>>> - clk_disable_unprepare(priv->bus_clk);
>>> - clk_disable_unprepare(priv->can_clk);
>>> +
>>> + pm_runtime_put(&pdev->dev);
>>> +
>>> netdev_dbg(ndev, "reg_base=0x%p irq=%d clock=%d, tx fifo
>> depth:%d\n",
>>> priv->reg_base, ndev->irq, priv->can.clock.freq,
>>> priv->tx_max);
>>>
>>
>> I think you have to convert the _remove() function, too. Have a look at the
>> gpio-zynq.c driver:
>>
>>> static int zynq_gpio_remove(struct platform_device *pdev) {
>>> struct zynq_gpio *gpio = platform_get_drvdata(pdev);
>>>
>>> pm_runtime_get_sync(&pdev->dev);
>>
>> However I don't understand why the get_sync() is here. Maybe Sören can
>> help?
>
> I converted the remove function to use the run-time PM and .
> Below is the remove code snippet.
>
> ret = pm_runtime_get_sync(&pdev->dev);
> if (ret < 0) {
> netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
> __func__, ret);
> return ret;
> }
>
> if (set_reset_mode(ndev) < 0)
> netdev_err(ndev, "mode resetting failed!\n");
>
> unregister_candev(ndev);
> netif_napi_del(&priv->napi);
> free_candev(ndev);
> pm_runtime_disable(&pdev->dev);
Can this make a call to xcan_runtime_*()? I'm asking since the ndev has
been unregistered and already free()ed. Better move this directly after
the set_reset_mode(). This way you are symmetric to the probe() function.
> Observed the below things in the /sys/kernel/debug/clk/clk_summary.
>
> Modprobe ifconfig up ifconfig down rmmod Modprobe ifconfig up ifconfig down rmmod Modprobe ifconfig up ifconfig down rmmod
> Clk_prepare count: 1 1 1 1 2 2 2 2 3 3 3 3
> Clk_enable count: 0 1 0 1 2 2 2 2 3 3 3 3
As the numbers are increasing you have a clk_prepare_enable() leak. Your
remove() function is missing the clk_disable_unprepare() for the can and
bus clock (as you have clk_prepare_enable in probe()).
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [iproute2 PATCH 1/1] actions: Get vlan action to work in pipeline
From: Jamal Hadi Salim @ 2015-01-11 14:31 UTC (permalink / raw)
To: stephen; +Cc: jiri, netdev, Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu.com>
When specified in a graph such as:
action vlan ... action foobar
the vlan action chewed more than it can swallow
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
tc/m_vlan.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/tc/m_vlan.c b/tc/m_vlan.c
index 171d268..32db5ed 100644
--- a/tc/m_vlan.c
+++ b/tc/m_vlan.c
@@ -103,20 +103,25 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
if (argc) {
if (matches(*argv, "reclassify") == 0) {
parm.action = TC_ACT_RECLASSIFY;
- NEXT_ARG();
+ argc--;
+ argv++;
} else if (matches(*argv, "pipe") == 0) {
parm.action = TC_ACT_PIPE;
- NEXT_ARG();
+ argc--;
+ argv++;
} else if (matches(*argv, "drop") == 0 ||
matches(*argv, "shot") == 0) {
parm.action = TC_ACT_SHOT;
- NEXT_ARG();
+ argc--;
+ argv++;
} else if (matches(*argv, "continue") == 0) {
parm.action = TC_ACT_UNSPEC;
- NEXT_ARG();
+ argc--;
+ argv++;
} else if (matches(*argv, "pass") == 0) {
parm.action = TC_ACT_OK;
- NEXT_ARG();
+ argc--;
+ argv++;
}
}
@@ -198,6 +203,7 @@ static int print_vlan(struct action_util *au, FILE *f, struct rtattr *arg)
}
break;
}
+ fprintf(f, " %s", action_n2a(parm->action, b1, sizeof (b1)));
fprintf(f, "\n\t index %d ref %d bind %d", parm->index, parm->refcnt,
parm->bindcnt);
--
1.7.9.5
^ permalink raw reply related
* [PATCH] net: sched: sch_teql: Remove unused function
From: Rickard Strandqvist @ 2015-01-11 14:08 UTC (permalink / raw)
To: Jamal Hadi Salim, David S. Miller
Cc: Rickard Strandqvist, netdev, linux-kernel
Remove the function teql_neigh_release() 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>
---
net/sched/sch_teql.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 6ada423..4899d4a 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -122,13 +122,6 @@ teql_peek(struct Qdisc *sch)
return NULL;
}
-static inline void
-teql_neigh_release(struct neighbour *n)
-{
- if (n)
- neigh_release(n);
-}
-
static void
teql_reset(struct Qdisc *sch)
{
--
1.7.10.4
^ permalink raw reply related
* Re: /proc/net/dev regression
From: Carlos R. Mafra @ 2015-01-11 13:40 UTC (permalink / raw)
To: Al Viro; +Cc: LKML, Hauke Mehrtens, John W. Linville, netdev
In-Reply-To: <20150111013913.GE22149@ZenIV.linux.org.uk>
On Sun, 11 Jan 2015 at 1:39:13 +0000, Al Viro wrote:
> On Sun, Jan 11, 2015 at 01:33:35AM +0000, Carlos R. Mafra wrote:
>
> > I think the problem with wmnet is not that it was expecting the fields
> > to be aligned because it never had problems before (when definitely more
> > than 10 megabytes were received, wmnet is crappy but not _that_ crappy).
> >
> > I think the problem really was here,
> >
> > totalbytes_in = strtoul(&buffer[7], NULL, 10);
> >
> > After the patch the device name is 8 characters long and &buffer[7]
> > overlaps with the name instead of reading the bytes. Before the
> > patch is was fine because the call to strtoul() seems correct in the
> > sense that it would read everything until the NULL. So more than 10
> > megabytes was still ok.
> >
> > So I guess I was wrong when suggesting that the problem was the
> > alignment.
>
> Several lines below there's this:
> totalpackets_out = strtoul(&buffer[74], NULL, 10);
> if (totalpackets_out != lastpackets_out) {
> totalbytes_out = strtoul(&buffer[66], NULL, 10);
> diffpackets_out += totalpackets_out - lastpackets_out;
> diffbytes_out += totalbytes_out - lastbytes_out;
> lastpackets_out = totalpackets_out;
> lastbytes_out = totalbytes_out;
> tx = True;
> }
>
> So I'm afraid it *is* that crappy. This function really should use scanf();
> note that updateStats_ipchains() in the same file does just that (well,
> fgets()+sscanf() for fsck knows what reason). And I'd be careful with all
> those %d, actually - it's not _that_ hard to get more than 4Gb sent.
> scanf formats really ought to match the kernel-side (seq_)printf one...
Ok, I fixed wmnet using Al's suggestion.
As far as I'm concerned, my regression complaint can be dismissed. It's
all working fine again.
Thanks!
^ permalink raw reply
* [PATCH] net: xfrm: xfrm_algo: Remove unused function
From: Rickard Strandqvist @ 2015-01-11 13:03 UTC (permalink / raw)
To: Steffen Klassert, Herbert Xu
Cc: Rickard Strandqvist, David S. Miller, netdev, linux-kernel
Remove the function aead_entries() 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>
---
net/xfrm/xfrm_algo.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/xfrm/xfrm_algo.c b/net/xfrm/xfrm_algo.c
index debe733..12e82a5 100644
--- a/net/xfrm/xfrm_algo.c
+++ b/net/xfrm/xfrm_algo.c
@@ -561,11 +561,6 @@ static struct xfrm_algo_desc calg_list[] = {
},
};
-static inline int aead_entries(void)
-{
- return ARRAY_SIZE(aead_list);
-}
-
static inline int aalg_entries(void)
{
return ARRAY_SIZE(aalg_list);
--
1.7.10.4
^ permalink raw reply related
* [net-next PATCH 1/1] net: sched: Introduce connmark action
From: Jamal Hadi Salim @ 2015-01-11 12:53 UTC (permalink / raw)
To: davem; +Cc: nbd, pablo, fw, netdev, Jamal Hadi Salim
From: Felix Fietkau <nbd@openwrt.org>
This tc action allows you to retrieve the connection tracking mark
There are known limitations currently:
doesn't work for inital packets, since we only query the ct table.
Fine given use case is for returning packets
no implicit defrag.
frags should be rare so fix later and what is a frag between friends
won't work for more complex tasks, e.g. lookup of other extensions
since we have no means to store results
we still have a 2nd lookup later on via normal conntrack path.
This shouldn't break anything though since skb->nfct isn't altered.
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
include/net/tc_act/tc_connmark.h | 14 ++
include/uapi/linux/tc_act/tc_connmark.h | 22 ++++
net/sched/Kconfig | 11 ++
net/sched/Makefile | 1 +
net/sched/act_connmark.c | 211 +++++++++++++++++++++++++++++++
5 files changed, 259 insertions(+)
create mode 100644 include/net/tc_act/tc_connmark.h
create mode 100644 include/uapi/linux/tc_act/tc_connmark.h
create mode 100644 net/sched/act_connmark.c
diff --git a/include/net/tc_act/tc_connmark.h b/include/net/tc_act/tc_connmark.h
new file mode 100644
index 0000000..5c1104c
--- /dev/null
+++ b/include/net/tc_act/tc_connmark.h
@@ -0,0 +1,14 @@
+#ifndef __NET_TC_CONNMARK_H
+#define __NET_TC_CONNMARK_H
+
+#include <net/act_api.h>
+
+struct tcf_connmark_info {
+ struct tcf_common common;
+ u16 zone;
+};
+
+#define to_connmark(a) \
+ container_of(a->priv, struct tcf_connmark_info, common)
+
+#endif /* __NET_TC_CONNMARK_H */
diff --git a/include/uapi/linux/tc_act/tc_connmark.h b/include/uapi/linux/tc_act/tc_connmark.h
new file mode 100644
index 0000000..82eda46
--- /dev/null
+++ b/include/uapi/linux/tc_act/tc_connmark.h
@@ -0,0 +1,22 @@
+#ifndef __UAPI_TC_CONNMARK_H
+#define __UAPI_TC_CONNMARK_H
+
+#include <linux/types.h>
+#include <linux/pkt_cls.h>
+
+#define TCA_ACT_CONNMARK 20
+
+struct tc_connmark {
+ tc_gen;
+ __u16 zone;
+};
+
+enum {
+ TCA_CONNMARK_UNSPEC,
+ TCA_CONNMARK_PARMS,
+ TCA_CONNMARK_TM,
+ __TCA_CONNMARK_MAX
+};
+#define TCA_CONNMARK_MAX (__TCA_CONNMARK_MAX - 1)
+
+#endif
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index c54c9d9..db20cae 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -698,6 +698,17 @@ config NET_ACT_VLAN
To compile this code as a module, choose M here: the
module will be called act_vlan.
+config NET_ACT_CONNMARK
+ tristate "Netfilter Connection Mark Retriever"
+ depends on NET_CLS_ACT && NETFILTER && IP_NF_IPTABLES
+ ---help---
+ Say Y here to allow retrieving of conn mark
+
+ If unsure, say N.
+
+ To compile this code as a module, choose M here: the
+ module will be called act_connmark.
+
config NET_CLS_IND
bool "Incoming device classification"
depends on NET_CLS_U32 || NET_CLS_FW
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 679f24a..47304cd 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_NET_ACT_SIMP) += act_simple.o
obj-$(CONFIG_NET_ACT_SKBEDIT) += act_skbedit.o
obj-$(CONFIG_NET_ACT_CSUM) += act_csum.o
obj-$(CONFIG_NET_ACT_VLAN) += act_vlan.o
+obj-$(CONFIG_NET_ACT_CONNMARK) += act_connmark.o
obj-$(CONFIG_NET_SCH_FIFO) += sch_fifo.o
obj-$(CONFIG_NET_SCH_CBQ) += sch_cbq.o
obj-$(CONFIG_NET_SCH_HTB) += sch_htb.o
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
new file mode 100644
index 0000000..f936434
--- /dev/null
+++ b/net/sched/act_connmark.c
@@ -0,0 +1,211 @@
+/*
+ * net/sched/act_connmark.c netfilter connmark retriever action
+ * skb mark is over-written
+ *
+ * Copyright (c) 2011 Felix Fietkau <nbd@openwrt.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ *
+ *
+*/
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/skbuff.h>
+#include <linux/rtnetlink.h>
+#include <linux/pkt_cls.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <net/netlink.h>
+#include <net/pkt_sched.h>
+#include <net/act_api.h>
+#include <uapi/linux/tc_act/tc_connmark.h>
+#include <net/tc_act/tc_connmark.h>
+
+#include <net/netfilter/nf_conntrack.h>
+#include <net/netfilter/nf_conntrack_core.h>
+#include <net/netfilter/nf_conntrack_zones.h>
+
+#define CONNMARK_TAB_MASK 3
+
+static struct tcf_hashinfo connmark_hash_info;
+
+static int tcf_connmark(struct sk_buff *skb, const struct tc_action *a,
+ struct tcf_result *res)
+{
+ const struct nf_conntrack_tuple_hash *thash;
+ struct nf_conntrack_tuple tuple;
+ enum ip_conntrack_info ctinfo;
+ struct tcf_connmark_info *ca = a->priv;
+ struct nf_conn *c;
+ int proto;
+
+ spin_lock(&ca->tcf_lock);
+ ca->tcf_tm.lastuse = jiffies;
+ bstats_update(&ca->tcf_bstats, skb);
+
+ if (skb->protocol == htons(ETH_P_IP)) {
+ if (skb->len < sizeof(struct iphdr)) {
+ goto out;
+ }
+ proto = NFPROTO_IPV4;
+ } else if (skb->protocol == htons(ETH_P_IPV6)) {
+ if (skb->len < sizeof(struct ipv6hdr)) {
+ goto out;
+ }
+ proto = NFPROTO_IPV6;
+ } else {
+ goto out;
+ }
+
+ c = nf_ct_get(skb, &ctinfo);
+ if (c != NULL) {
+ skb->mark = c->mark;
+ /* using overlimits stats to count how many packets marked */
+ ca->tcf_qstats.overlimits++;
+ nf_ct_put(c);
+ goto out;
+ }
+
+ if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
+ proto, &tuple))
+ goto out;
+
+ thash = nf_conntrack_find_get(dev_net(skb->dev), ca->zone, &tuple);
+ if (!thash)
+ goto out;
+
+ c = nf_ct_tuplehash_to_ctrack(thash);
+ /* using overlimits stats to count how many packets marked */
+ ca->tcf_qstats.overlimits++;
+ skb->mark = c->mark;
+ nf_ct_put(c);
+
+out:
+ skb->nfct = NULL;
+ spin_unlock(&ca->tcf_lock);
+ return ca->tcf_action;
+}
+
+static const struct nla_policy connmark_policy[TCA_CONNMARK_MAX + 1] = {
+ [TCA_CONNMARK_PARMS] = { .len = sizeof(struct tc_connmark) },
+};
+
+static int tcf_connmark_init(struct net *net, struct nlattr *nla,
+ struct nlattr *est, struct tc_action *a,
+ int ovr, int bind)
+{
+ struct nlattr *tb[TCA_CONNMARK_MAX + 1];
+ struct tcf_connmark_info *ci;
+ struct tc_connmark *parm;
+ int ret = 0;
+
+ if (nla == NULL)
+ return -EINVAL;
+
+ ret = nla_parse_nested(tb, TCA_CONNMARK_MAX, nla, connmark_policy);
+ if (ret < 0)
+ return ret;
+
+ parm = nla_data(tb[TCA_CONNMARK_PARMS]);
+
+ if (!tcf_hash_check(parm->index, a, bind)) {
+ ret = tcf_hash_create(parm->index, est, a, sizeof(*ci), bind);
+ if (ret)
+ return ret;
+
+ ci = to_connmark(a);
+ ci->tcf_action = parm->action;
+ ci->zone = parm->zone;
+
+ tcf_hash_insert(a);
+ ret = ACT_P_CREATED;
+ } else {
+ ci = to_connmark(a);
+ if (bind)
+ return 0;
+ tcf_hash_release(a, bind);
+ if (!ovr)
+ return -EEXIST;
+ /* replacing action and zone */
+ ci->tcf_action = parm->action;
+ ci->zone = parm->zone;
+ }
+
+ return ret;
+}
+
+static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
+ int bind, int ref)
+{
+ unsigned char *b = skb_tail_pointer(skb);
+ struct tcf_connmark_info *ci = a->priv;
+
+ struct tc_connmark opt = {
+ .index = ci->tcf_index,
+ .refcnt = ci->tcf_refcnt - ref,
+ .bindcnt = ci->tcf_bindcnt - bind,
+ .action = ci->tcf_action,
+ .zone = ci->zone,
+ };
+ struct tcf_t t;
+
+ if (nla_put(skb, TCA_CONNMARK_PARMS, sizeof(opt), &opt))
+ goto nla_put_failure;
+
+ t.install = jiffies_to_clock_t(jiffies - ci->tcf_tm.install);
+ t.lastuse = jiffies_to_clock_t(jiffies - ci->tcf_tm.lastuse);
+ t.expires = jiffies_to_clock_t(ci->tcf_tm.expires);
+ if (nla_put(skb, TCA_CONNMARK_TM, sizeof(t), &t))
+ goto nla_put_failure;
+
+ return skb->len;
+nla_put_failure:
+ nlmsg_trim(skb, b);
+ return -1;
+}
+
+static struct tc_action_ops act_connmark_ops = {
+ .kind = "connmark",
+ .hinfo = &connmark_hash_info,
+ .type = TCA_ACT_CONNMARK,
+ .owner = THIS_MODULE,
+ .act = tcf_connmark,
+ .dump = tcf_connmark_dump,
+ .init = tcf_connmark_init,
+};
+
+MODULE_AUTHOR("Felix Fietkau <nbd@openwrt.org>");
+MODULE_DESCRIPTION("Connection tracking mark restoring");
+MODULE_LICENSE("GPL");
+
+static int __init connmark_init_module(void)
+{
+ int ret;
+
+ ret = tcf_hashinfo_init(&connmark_hash_info, CONNMARK_TAB_MASK);
+ if (ret)
+ return ret;
+
+ return tcf_register_action(&act_connmark_ops, CONNMARK_TAB_MASK);
+}
+
+static void __exit connmark_cleanup_module(void)
+{
+ tcf_unregister_action(&act_connmark_ops);
+}
+
+module_init(connmark_init_module);
+module_exit(connmark_cleanup_module);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 1/3] rtlwifi: btcoexist: Remove some unused functions
From: Rickard Strandqvist @ 2015-01-11 11:56 UTC (permalink / raw)
To: Larry Finger
Cc: Kalle Valo, Masanari Iida, Sachin Kamat,
linux-wireless@vger.kernel.org, Network Development,
Linux Kernel Mailing List
In-Reply-To: <54B15D57.5060709@lwfinger.net>
2015-01-10 18:11 GMT+01:00 Larry Finger <Larry.Finger@lwfinger.net>:
> On 01/10/2015 10:24 AM, Rickard Strandqvist wrote:
>>
>> Removes some functions that are not used anywhere:
>> ex_halbtc8821a1ant_periodical() ex_halbtc8821a1ant_pnp_notify()
>> ex_halbtc8821a1ant_halt_notify() ex_halbtc8821a1ant_bt_info_notify()
>> ex_halbtc8821a1ant_special_packet_notify()
>> ex_halbtc8821a1ant_connect_notify()
>> ex_halbtc8821a1ant_scan_notify() ex_halbtc8821a1ant_lps_notify()
>> ex_halbtc8821a1ant_ips_notify() ex_halbtc8821a1ant_display_coex_info()
>> ex_halbtc8821a1ant_init_coex_dm() ex_halbtc8821a1ant_init_hwconfig()
>>
>> This was partially found by using a static code analysis program called
>> cppcheck.
>>
>> Signed-off-by: Rickard Strandqvist
>> <rickard_strandqvist@spectrumdigital.se>
>
>
> NACK!!!!!!!!!
>
> I told you to stay away from these routines until I finish my update. Not
> only might you remove some functions that I will be needing later, but you
> run the risk of merge complications.
>
> Kalle: Please ignore EVERYTHING from this person. Obviously, he is incapable
> of understanding even the simplest instructions.
>
> Larry
>
>
>> ---
>> .../wireless/rtlwifi/btcoexist/halbtc8821a1ant.c | 707
>> --------------------
>> .../wireless/rtlwifi/btcoexist/halbtc8821a1ant.h | 14 -
>> 2 files changed, 721 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.c
>> b/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.c
>> index b72e537..a86e6b6 100644
>> --- a/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.c
>> +++ b/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.c
>> @@ -2213,435 +2213,6 @@ static void halbtc8821a1ant_init_hw_config(struct
>> btc_coexist *btcoexist,
>> /*============================================================*/
>> /* extern function start with EXhalbtc8821a1ant_*/
>> /*============================================================*/
>> -void ex_halbtc8821a1ant_init_hwconfig(struct btc_coexist *btcoexist)
>> -{
>> - halbtc8821a1ant_init_hw_config(btcoexist, true);
>> -}
>> -
>> -void ex_halbtc8821a1ant_init_coex_dm(struct btc_coexist *btcoexist)
>> -{
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex], Coex Mechanism Init!!\n");
>> -
>> - btcoexist->stop_coex_dm = false;
>> -
>> - halbtc8821a1ant_init_coex_dm(btcoexist);
>> -
>> - halbtc8821a1ant_query_bt_info(btcoexist);
>> -}
>> -
>> -void ex_halbtc8821a1ant_display_coex_info(struct btc_coexist *btcoexist)
>> -{
>> - struct btc_board_info *board_info = &btcoexist->board_info;
>> - struct btc_stack_info *stack_info = &btcoexist->stack_info;
>> - struct btc_bt_link_info *bt_link_info = &btcoexist->bt_link_info;
>> - struct rtl_priv *rtlpriv = btcoexist->adapter;
>> - u8 u1_tmp[4], i, bt_info_ext, ps_tdma_case = 0;
>> - u16 u2_tmp[4];
>> - u32 u4_tmp[4];
>> - bool roam = false, scan = false, link = false, wifi_under_5g =
>> false;
>> - bool bt_hs_on = false, wifi_busy = false;
>> - long wifi_rssi = 0, bt_hs_rssi = 0;
>> - u32 wifi_bw, wifi_traffic_dir;
>> - u8 wifi_dot11_chnl, wifi_hs_chnl;
>> - u32 fw_ver = 0, bt_patch_ver = 0;
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n ============[BT Coexist info]============");
>> -
>> - if (btcoexist->manual_control) {
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n ============[Under Manual
>> Control]============");
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n
>> ==========================================");
>> - }
>> - if (btcoexist->stop_coex_dm) {
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n ============[Coex is
>> STOPPED]============");
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n
>> ==========================================");
>> - }
>> -
>> - if (!board_info->bt_exist) {
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG, "\r\n BT not
>> exists !!!");
>> - return;
>> - }
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d/ %d/ %d",
>> - "Ant PG Num/ Ant Mech/ Ant Pos:",
>> - board_info->pg_ant_num,
>> - board_info->btdm_ant_num,
>> - board_info->btdm_ant_pos);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %s / %d", "BT stack/ hci ext ver",
>> - ((stack_info->profile_notified) ? "Yes" : "No"),
>> - stack_info->hci_version);
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_BT_PATCH_VER,
>> - &bt_patch_ver);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_WIFI_FW_VER, &fw_ver);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d_%x/ 0x%x/ 0x%x(%d)",
>> - "CoexVer/ FwVer/ PatchVer",
>> - glcoex_ver_date_8821a_1ant,
>> - glcoex_ver_8821a_1ant,
>> - fw_ver, bt_patch_ver,
>> - bt_patch_ver);
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_HS_OPERATION,
>> - &bt_hs_on);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U1_WIFI_DOT11_CHNL,
>> - &wifi_dot11_chnl);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U1_WIFI_HS_CHNL,
>> - &wifi_hs_chnl);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d / %d(%d)",
>> - "Dot11 channel / HsChnl(HsMode)",
>> - wifi_dot11_chnl, wifi_hs_chnl, bt_hs_on);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %02x %02x %02x ",
>> - "H2C Wifi inform bt chnl Info",
>> - coex_dm->wifi_chnl_info[0], coex_dm->wifi_chnl_info[1],
>> - coex_dm->wifi_chnl_info[2]);
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_S4_WIFI_RSSI, &wifi_rssi);
>> - btcoexist->btc_get(btcoexist, BTC_GET_S4_HS_RSSI, &bt_hs_rssi);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d/ %d", "Wifi rssi/ HS rssi",
>> - (int)wifi_rssi, (int)bt_hs_rssi);
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_SCAN, &scan);
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_LINK, &link);
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_ROAM, &roam);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d/ %d/ %d ", "Wifi link/ roam/ scan",
>> - link, roam, scan);
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_UNDER_5G,
>> - &wifi_under_5g);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_WIFI_BW,
>> - &wifi_bw);
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_BUSY,
>> - &wifi_busy);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_WIFI_TRAFFIC_DIRECTION,
>> - &wifi_traffic_dir);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %s / %s/ %s ", "Wifi status",
>> - (wifi_under_5g ? "5G" : "2.4G"),
>> - ((BTC_WIFI_BW_LEGACY == wifi_bw) ? "Legacy" :
>> - (((BTC_WIFI_BW_HT40 == wifi_bw) ? "HT40" : "HT20"))),
>> - ((!wifi_busy) ? "idle" :
>> - ((BTC_WIFI_TRAFFIC_TX == wifi_traffic_dir) ?
>> - "uplink" : "downlink")));
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = [%s/ %d/ %d] ", "BT [status/ rssi/
>> retryCnt]",
>> - ((btcoexist->bt_info.bt_disabled) ? ("disabled") :
>> - ((coex_sta->c2h_bt_inquiry_page) ? ("inquiry/page
>> scan") :
>> - ((BT_8821A_1ANT_BT_STATUS_NON_CONNECTED_IDLE ==
>> - coex_dm->bt_status) ?
>> - "non-connected idle" :
>> - ((BT_8821A_1ANT_BT_STATUS_CONNECTED_IDLE ==
>> - coex_dm->bt_status) ?
>> - "connected-idle" : "busy")))),
>> - coex_sta->bt_rssi, coex_sta->bt_retry_cnt);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d / %d / %d / %d", "SCO/HID/PAN/A2DP",
>> - bt_link_info->sco_exist,
>> - bt_link_info->hid_exist,
>> - bt_link_info->pan_exist,
>> - bt_link_info->a2dp_exist);
>> - btcoexist->btc_disp_dbg_msg(btcoexist, BTC_DBG_DISP_BT_LINK_INFO);
>> -
>> - bt_info_ext = coex_sta->bt_info_ext;
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %s",
>> - "BT Info A2DP rate",
>> - (bt_info_ext&BIT0) ?
>> - "Basic rate" : "EDR rate");
>> -
>> - for (i = 0; i < BT_INFO_SRC_8821A_1ANT_MAX; i++) {
>> - if (coex_sta->bt_info_c2h_cnt[i]) {
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %02x %02x %02x %02x %02x
>> %02x %02x(%d)",
>> - glbt_info_src_8821a_1ant[i],
>> - coex_sta->bt_info_c2h[i][0],
>> - coex_sta->bt_info_c2h[i][1],
>> - coex_sta->bt_info_c2h[i][2],
>> - coex_sta->bt_info_c2h[i][3],
>> - coex_sta->bt_info_c2h[i][4],
>> - coex_sta->bt_info_c2h[i][5],
>> - coex_sta->bt_info_c2h[i][6],
>> - coex_sta->bt_info_c2h_cnt[i]);
>> - }
>> - }
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %s/%s, (0x%x/0x%x)",
>> - "PS state, IPS/LPS, (lps/rpwm)",
>> - ((coex_sta->under_ips ? "IPS ON" : "IPS OFF")),
>> - ((coex_sta->under_Lps ? "LPS ON" : "LPS OFF")),
>> - btcoexist->bt_info.lps_val,
>> - btcoexist->bt_info.rpwm_val);
>> - btcoexist->btc_disp_dbg_msg(btcoexist,
>> BTC_DBG_DISP_FW_PWR_MODE_CMD);
>> -
>> - if (!btcoexist->manual_control) {
>> - /* Sw mechanism*/
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s", "============[Sw
>> mechanism]============");
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d", "SM[LowPenaltyRA]",
>> - coex_dm->cur_low_penalty_ra);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %s/ %s/ %d ",
>> - "DelBA/ BtCtrlAgg/ AggSize",
>> - (btcoexist->bt_info.reject_agg_pkt ? "Yes" :
>> "No"),
>> - (btcoexist->bt_info.bt_ctrl_buf_size ? "Yes" :
>> "No"),
>> - btcoexist->bt_info.agg_buf_size);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x ", "Rate Mask",
>> - btcoexist->bt_info.ra_mask);
>> -
>> - /* Fw mechanism*/
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG, "\r\n %-35s",
>> - "============[Fw mechanism]============");
>> -
>> - ps_tdma_case = coex_dm->cur_ps_tdma;
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %02x %02x %02x %02x %02x case-%d
>> (auto:%d)",
>> - "PS TDMA",
>> - coex_dm->ps_tdma_para[0],
>> - coex_dm->ps_tdma_para[1],
>> - coex_dm->ps_tdma_para[2],
>> - coex_dm->ps_tdma_para[3],
>> - coex_dm->ps_tdma_para[4],
>> - ps_tdma_case,
>> - coex_dm->auto_tdma_adjust);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x ",
>> - "Latest error condition(should be 0)",
>> - coex_dm->error_condition);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d ", "IgnWlanAct",
>> - coex_dm->cur_ignore_wlan_act);
>> - }
>> -
>> - /* Hw setting*/
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s", "============[Hw setting]============");
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/0x%x/0x%x/0x%x",
>> - "backup ARFR1/ARFR2/RL/AMaxTime",
>> - coex_dm->backup_arfr_cnt1,
>> - coex_dm->backup_arfr_cnt2,
>> - coex_dm->backup_retry_limit,
>> - coex_dm->backup_ampdu_max_time);
>> -
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0x430);
>> - u4_tmp[1] = btcoexist->btc_read_4byte(btcoexist, 0x434);
>> - u2_tmp[0] = btcoexist->btc_read_2byte(btcoexist, 0x42a);
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x456);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/0x%x/0x%x/0x%x",
>> - "0x430/0x434/0x42a/0x456",
>> - u4_tmp[0], u4_tmp[1], u2_tmp[0], u1_tmp[0]);
>> -
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x778);
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0xc58);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x", "0x778/ 0xc58[29:25]",
>> - u1_tmp[0], (u4_tmp[0]&0x3e000000) >> 25);
>> -
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x8db);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x", "0x8db[6:5]",
>> - ((u1_tmp[0]&0x60)>>5));
>> -
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x975);
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0xcb4);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x/ 0x%x",
>> - "0xcb4[29:28]/0xcb4[7:0]/0x974[9:8]",
>> - (u4_tmp[0] & 0x30000000)>>28,
>> - u4_tmp[0] & 0xff,
>> - u1_tmp[0] & 0x3);
>> -
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x40);
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0x4c);
>> - u1_tmp[1] = btcoexist->btc_read_1byte(btcoexist, 0x64);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x/ 0x%x",
>> - "0x40/0x4c[24:23]/0x64[0]",
>> - u1_tmp[0], ((u4_tmp[0]&0x01800000)>>23),
>> u1_tmp[1]&0x1);
>> -
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0x550);
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x522);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x", "0x550(bcn ctrl)/0x522",
>> - u4_tmp[0], u1_tmp[0]);
>> -
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0xc50);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x", "0xc50(dig)",
>> - u4_tmp[0]&0xff);
>> -
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0xf48);
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0xa5d);
>> - u1_tmp[1] = btcoexist->btc_read_1byte(btcoexist, 0xa5c);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x", "OFDM-FA/ CCK-FA",
>> - u4_tmp[0], (u1_tmp[0]<<8) + u1_tmp[1]);
>> -
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0x6c0);
>> - u4_tmp[1] = btcoexist->btc_read_4byte(btcoexist, 0x6c4);
>> - u4_tmp[2] = btcoexist->btc_read_4byte(btcoexist, 0x6c8);
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x6cc);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x/ 0x%x/ 0x%x",
>> - "0x6c0/0x6c4/0x6c8/0x6cc(coexTable)",
>> - u4_tmp[0], u4_tmp[1], u4_tmp[2], u1_tmp[0]);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d/ %d", "0x770(high-pri rx/tx)",
>> - coex_sta->high_priority_rx,
>> coex_sta->high_priority_tx);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d/ %d", "0x774(low-pri rx/tx)",
>> - coex_sta->low_priority_rx, coex_sta->low_priority_tx);
>> -#if (BT_AUTO_REPORT_ONLY_8821A_1ANT == 1)
>> - halbtc8821a1ant_monitor_bt_ctr(btcoexist);
>> -#endif
>> - btcoexist->btc_disp_dbg_msg(btcoexist,
>> BTC_DBG_DISP_COEX_STATISTICS);
>> -}
>> -
>> -void ex_halbtc8821a1ant_ips_notify(struct btc_coexist *btcoexist, u8
>> type)
>> -{
>> - if (btcoexist->manual_control || btcoexist->stop_coex_dm)
>> - return;
>> -
>> - if (BTC_IPS_ENTER == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], IPS ENTER notify\n");
>> - coex_sta->under_ips = true;
>> - halbtc8821a1ant_set_ant_path(btcoexist,
>> - BTC_ANT_PATH_BT, false,
>> true);
>> - /*set PTA control*/
>> - halbtc8821a1ant_ps_tdma(btcoexist, NORMAL_EXEC, false, 8);
>> - halbtc8821a1ant_coex_table_with_type(btcoexist,
>> - NORMAL_EXEC, 0);
>> - } else if (BTC_IPS_LEAVE == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], IPS LEAVE notify\n");
>> - coex_sta->under_ips = false;
>> -
>> - halbtc8821a1ant_run_coexist_mechanism(btcoexist);
>> - }
>> -}
>> -
>> -void ex_halbtc8821a1ant_lps_notify(struct btc_coexist *btcoexist, u8
>> type)
>> -{
>> - if (btcoexist->manual_control || btcoexist->stop_coex_dm)
>> - return;
>> -
>> - if (BTC_LPS_ENABLE == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], LPS ENABLE notify\n");
>> - coex_sta->under_Lps = true;
>> - } else if (BTC_LPS_DISABLE == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], LPS DISABLE notify\n");
>> - coex_sta->under_Lps = false;
>> - }
>> -}
>> -
>> -void ex_halbtc8821a1ant_scan_notify(struct btc_coexist *btcoexist, u8
>> type)
>> -{
>> - bool wifi_connected = false, bt_hs_on = false;
>> -
>> - if (btcoexist->manual_control ||
>> - btcoexist->stop_coex_dm ||
>> - btcoexist->bt_info.bt_disabled)
>> - return;
>> -
>> - btcoexist->btc_get(btcoexist,
>> - BTC_GET_BL_HS_OPERATION, &bt_hs_on);
>> - btcoexist->btc_get(btcoexist,
>> - BTC_GET_BL_WIFI_CONNECTED, &wifi_connected);
>> -
>> - halbtc8821a1ant_query_bt_info(btcoexist);
>> -
>> - if (coex_sta->c2h_bt_inquiry_page) {
>> - halbtc8821a1ant_action_bt_inquiry(btcoexist);
>> - return;
>> - } else if (bt_hs_on) {
>> - halbtc8821a1ant_action_hs(btcoexist);
>> - return;
>> - }
>> -
>> - if (BTC_SCAN_START == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], SCAN START notify\n");
>> - if (!wifi_connected) {
>> - /* non-connected scan*/
>> - btc8821a1ant_act_wifi_not_conn_scan(btcoexist);
>> - } else {
>> - /* wifi is connected*/
>> -
>> halbtc8821a1ant_action_wifi_connected_scan(btcoexist);
>> - }
>> - } else if (BTC_SCAN_FINISH == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], SCAN FINISH notify\n");
>> - if (!wifi_connected) {
>> - /* non-connected scan*/
>> -
>> halbtc8821a1ant_action_wifi_not_connected(btcoexist);
>> - } else {
>> - halbtc8821a1ant_action_wifi_connected(btcoexist);
>> - }
>> - }
>> -}
>> -
>> -void ex_halbtc8821a1ant_connect_notify(struct btc_coexist *btcoexist, u8
>> type)
>> -{
>> - bool wifi_connected = false, bt_hs_on = false;
>> -
>> - if (btcoexist->manual_control ||
>> - btcoexist->stop_coex_dm ||
>> - btcoexist->bt_info.bt_disabled)
>> - return;
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_HS_OPERATION, &bt_hs_on);
>> - if (coex_sta->c2h_bt_inquiry_page) {
>> - halbtc8821a1ant_action_bt_inquiry(btcoexist);
>> - return;
>> - } else if (bt_hs_on) {
>> - halbtc8821a1ant_action_hs(btcoexist);
>> - return;
>> - }
>> -
>> - if (BTC_ASSOCIATE_START == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], CONNECT START notify\n");
>> - btc8821a1ant_act_wifi_not_conn_scan(btcoexist);
>> - } else if (BTC_ASSOCIATE_FINISH == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], CONNECT FINISH notify\n");
>> -
>> - btcoexist->btc_get(btcoexist,
>> - BTC_GET_BL_WIFI_CONNECTED, &wifi_connected);
>> - if (!wifi_connected) {
>> - /* non-connected scan*/
>> -
>> halbtc8821a1ant_action_wifi_not_connected(btcoexist);
>> - } else {
>> - halbtc8821a1ant_action_wifi_connected(btcoexist);
>> - }
>> - }
>> -}
>>
>> void ex_halbtc8821a1ant_media_status_notify(struct btc_coexist
>> *btcoexist,
>> u8 type)
>> @@ -2690,281 +2261,3 @@ void ex_halbtc8821a1ant_media_status_notify(struct
>> btc_coexist *btcoexist,
>> btcoexist->btc_fill_h2c(btcoexist, 0x66, 3, h2c_parameter);
>> }
>>
>> -void ex_halbtc8821a1ant_special_packet_notify(struct btc_coexist
>> *btcoexist,
>> - u8 type)
>> -{
>> - bool bt_hs_on = false;
>> -
>> - if (btcoexist->manual_control ||
>> - btcoexist->stop_coex_dm ||
>> - btcoexist->bt_info.bt_disabled)
>> - return;
>> -
>> - coex_sta->special_pkt_period_cnt = 0;
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_HS_OPERATION, &bt_hs_on);
>> - if (coex_sta->c2h_bt_inquiry_page) {
>> - halbtc8821a1ant_action_bt_inquiry(btcoexist);
>> - return;
>> - } else if (bt_hs_on) {
>> - halbtc8821a1ant_action_hs(btcoexist);
>> - return;
>> - }
>> -
>> - if (BTC_PACKET_DHCP == type ||
>> - BTC_PACKET_EAPOL == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], special Packet(%d) notify\n", type);
>> - btc8821a1ant_act_wifi_conn_sp_pkt(btcoexist);
>> - }
>> -}
>> -
>> -void ex_halbtc8821a1ant_bt_info_notify(struct btc_coexist *btcoexist,
>> - u8 *tmp_buf, u8 length)
>> -{
>> - u8 bt_info = 0;
>> - u8 i, rsp_source = 0;
>> - bool wifi_connected = false;
>> - bool bt_busy = false;
>> - bool wifi_under_5g = false;
>> -
>> - coex_sta->c2h_bt_info_req_sent = false;
>> -
>> - btcoexist->btc_get(btcoexist,
>> - BTC_GET_BL_WIFI_UNDER_5G, &wifi_under_5g);
>> -
>> - rsp_source = tmp_buf[0]&0xf;
>> - if (rsp_source >= BT_INFO_SRC_8821A_1ANT_MAX)
>> - rsp_source = BT_INFO_SRC_8821A_1ANT_WIFI_FW;
>> - coex_sta->bt_info_c2h_cnt[rsp_source]++;
>> -
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], Bt info[%d], length = %d, hex data = [",
>> - rsp_source, length);
>> - for (i = 0; i < length; i++) {
>> - coex_sta->bt_info_c2h[rsp_source][i] = tmp_buf[i];
>> - if (i == 1)
>> - bt_info = tmp_buf[i];
>> - if (i == length-1) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "0x%02x]\n", tmp_buf[i]);
>> - } else {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "0x%02x, ", tmp_buf[i]);
>> - }
>> - }
>> -
>> - if (BT_INFO_SRC_8821A_1ANT_WIFI_FW != rsp_source) {
>> - coex_sta->bt_retry_cnt = /* [3:0]*/
>> - coex_sta->bt_info_c2h[rsp_source][2]&0xf;
>> -
>> - coex_sta->bt_rssi =
>> - coex_sta->bt_info_c2h[rsp_source][3]*2+10;
>> -
>> - coex_sta->bt_info_ext =
>> - coex_sta->bt_info_c2h[rsp_source][4];
>> -
>> - /* Here we need to resend some wifi info to BT*/
>> - /* because bt is reset and loss of the info.*/
>> - if (coex_sta->bt_info_ext & BIT1) {
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BT ext info bit1 check, send
>> wifi BW&Chnl to BT!!\n");
>> - btcoexist->btc_get(btcoexist,
>> - BTC_GET_BL_WIFI_CONNECTED,
>> - &wifi_connected);
>> - if (wifi_connected) {
>> -
>> ex_halbtc8821a1ant_media_status_notify(btcoexist,
>> -
>> BTC_MEDIA_CONNECT);
>> - } else {
>> -
>> ex_halbtc8821a1ant_media_status_notify(btcoexist,
>> -
>> BTC_MEDIA_DISCONNECT);
>> - }
>> - }
>> -
>> - if ((coex_sta->bt_info_ext & BIT3) && !wifi_under_5g) {
>> - if (!btcoexist->manual_control &&
>> - !btcoexist->stop_coex_dm) {
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BT ext info bit3
>> check, set BT NOT to ignore Wlan active!!\n");
>> - halbtc8821a1ant_ignore_wlan_act(btcoexist,
>> -
>> FORCE_EXEC,
>> - false);
>> - }
>> - }
>> -#if (BT_AUTO_REPORT_ONLY_8821A_1ANT == 0)
>> - if (!(coex_sta->bt_info_ext & BIT4)) {
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BT ext info bit4 check, set
>> BT to enable Auto Report!!\n");
>> - halbtc8821a1ant_bt_auto_report(btcoexist,
>> - FORCE_EXEC, true);
>> - }
>> -#endif
>> - }
>> -
>> - /* check BIT2 first ==> check if bt is under inquiry or page
>> scan*/
>> - if (bt_info & BT_INFO_8821A_1ANT_B_INQ_PAGE)
>> - coex_sta->c2h_bt_inquiry_page = true;
>> - else
>> - coex_sta->c2h_bt_inquiry_page = false;
>> -
>> - /* set link exist status*/
>> - if (!(bt_info&BT_INFO_8821A_1ANT_B_CONNECTION)) {
>> - coex_sta->bt_link_exist = false;
>> - coex_sta->pan_exist = false;
>> - coex_sta->a2dp_exist = false;
>> - coex_sta->hid_exist = false;
>> - coex_sta->sco_exist = false;
>> - } else {
>> - /* connection exists*/
>> - coex_sta->bt_link_exist = true;
>> - if (bt_info & BT_INFO_8821A_1ANT_B_FTP)
>> - coex_sta->pan_exist = true;
>> - else
>> - coex_sta->pan_exist = false;
>> - if (bt_info & BT_INFO_8821A_1ANT_B_A2DP)
>> - coex_sta->a2dp_exist = true;
>> - else
>> - coex_sta->a2dp_exist = false;
>> - if (bt_info & BT_INFO_8821A_1ANT_B_HID)
>> - coex_sta->hid_exist = true;
>> - else
>> - coex_sta->hid_exist = false;
>> - if (bt_info & BT_INFO_8821A_1ANT_B_SCO_ESCO)
>> - coex_sta->sco_exist = true;
>> - else
>> - coex_sta->sco_exist = false;
>> - }
>> -
>> - halbtc8821a1ant_update_bt_link_info(btcoexist);
>> -
>> - if (!(bt_info&BT_INFO_8821A_1ANT_B_CONNECTION)) {
>> - coex_dm->bt_status =
>> BT_8821A_1ANT_BT_STATUS_NON_CONNECTED_IDLE;
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BtInfoNotify(), BT Non-Connected
>> idle!!!\n");
>> - } else if (bt_info == BT_INFO_8821A_1ANT_B_CONNECTION) {
>> - /* connection exists but no busy*/
>> - coex_dm->bt_status =
>> BT_8821A_1ANT_BT_STATUS_CONNECTED_IDLE;
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BtInfoNotify(), BT
>> Connected-idle!!!\n");
>> - } else if ((bt_info&BT_INFO_8821A_1ANT_B_SCO_ESCO) ||
>> - (bt_info&BT_INFO_8821A_1ANT_B_SCO_BUSY)) {
>> - coex_dm->bt_status = BT_8821A_1ANT_BT_STATUS_SCO_BUSY;
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BtInfoNotify(), BT SCO busy!!!\n");
>> - } else if (bt_info&BT_INFO_8821A_1ANT_B_ACL_BUSY) {
>> - if (BT_8821A_1ANT_BT_STATUS_ACL_BUSY !=
>> coex_dm->bt_status)
>> - coex_dm->auto_tdma_adjust = false;
>> - coex_dm->bt_status = BT_8821A_1ANT_BT_STATUS_ACL_BUSY;
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BtInfoNotify(), BT ACL busy!!!\n");
>> - } else {
>> - coex_dm->bt_status = BT_8821A_1ANT_BT_STATUS_MAX;
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BtInfoNotify(), BT Non-Defined
>> state!!!\n");
>> - }
>> -
>> - if ((BT_8821A_1ANT_BT_STATUS_ACL_BUSY == coex_dm->bt_status) ||
>> - (BT_8821A_1ANT_BT_STATUS_SCO_BUSY == coex_dm->bt_status) ||
>> - (BT_8821A_1ANT_BT_STATUS_ACL_SCO_BUSY == coex_dm->bt_status))
>> - bt_busy = true;
>> - else
>> - bt_busy = false;
>> - btcoexist->btc_set(btcoexist,
>> - BTC_SET_BL_BT_TRAFFIC_BUSY, &bt_busy);
>> -
>> - halbtc8821a1ant_run_coexist_mechanism(btcoexist);
>> -}
>> -
>> -void ex_halbtc8821a1ant_halt_notify(struct btc_coexist *btcoexist)
>> -{
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], Halt notify\n");
>> -
>> - btcoexist->stop_coex_dm = true;
>> -
>> - halbtc8821a1ant_set_ant_path(btcoexist,
>> - BTC_ANT_PATH_BT, false, true);
>> - halbtc8821a1ant_ignore_wlan_act(btcoexist, FORCE_EXEC, true);
>> -
>> - halbtc8821a1ant_power_save_state(btcoexist,
>> - BTC_PS_WIFI_NATIVE, 0x0, 0x0);
>> - halbtc8821a1ant_ps_tdma(btcoexist, FORCE_EXEC, false, 0);
>> -
>> - ex_halbtc8821a1ant_media_status_notify(btcoexist,
>> - BTC_MEDIA_DISCONNECT);
>> -}
>> -
>> -void ex_halbtc8821a1ant_pnp_notify(struct btc_coexist *btcoexist, u8
>> pnp_state)
>> -{
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], Pnp notify\n");
>> -
>> - if (BTC_WIFI_PNP_SLEEP == pnp_state) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], Pnp notify to SLEEP\n");
>> - btcoexist->stop_coex_dm = true;
>> - halbtc8821a1ant_ignore_wlan_act(btcoexist, FORCE_EXEC,
>> true);
>> - halbtc8821a1ant_power_save_state(btcoexist,
>> BTC_PS_WIFI_NATIVE,
>> - 0x0, 0x0);
>> - halbtc8821a1ant_ps_tdma(btcoexist, NORMAL_EXEC, false, 9);
>> - } else if (BTC_WIFI_PNP_WAKE_UP == pnp_state) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], Pnp notify to WAKE UP\n");
>> - btcoexist->stop_coex_dm = false;
>> - halbtc8821a1ant_init_hw_config(btcoexist, false);
>> - halbtc8821a1ant_init_coex_dm(btcoexist);
>> - halbtc8821a1ant_query_bt_info(btcoexist);
>> - }
>> -}
>> -
>> -void
>> -ex_halbtc8821a1ant_periodical(
>> - struct btc_coexist *btcoexist) {
>> - static u8 dis_ver_info_cnt;
>> - u32 fw_ver = 0, bt_patch_ver = 0;
>> - struct btc_board_info *board_info = &btcoexist->board_info;
>> - struct btc_stack_info *stack_info = &btcoexist->stack_info;
>> -
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex],
>> ==========================Periodical===========================\n");
>> -
>> - if (dis_ver_info_cnt <= 5) {
>> - dis_ver_info_cnt += 1;
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex],
>> ****************************************************************\n");
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex], Ant PG Num/ Ant Mech/ Ant Pos = %d/
>> %d/ %d\n",
>> - board_info->pg_ant_num,
>> - board_info->btdm_ant_num,
>> - board_info->btdm_ant_pos);
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex], BT stack/ hci ext ver = %s / %d\n",
>> - ((stack_info->profile_notified) ? "Yes" : "No"),
>> - stack_info->hci_version);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_BT_PATCH_VER,
>> - &bt_patch_ver);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_WIFI_FW_VER,
>> &fw_ver);
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex], CoexVer/ FwVer/ PatchVer = %d_%x/
>> 0x%x/ 0x%x(%d)\n",
>> - glcoex_ver_date_8821a_1ant,
>> - glcoex_ver_8821a_1ant,
>> - fw_ver, bt_patch_ver,
>> - bt_patch_ver);
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex],
>> ****************************************************************\n");
>> - }
>> -
>> -#if (BT_AUTO_REPORT_ONLY_8821A_1ANT == 0)
>> - halbtc8821a1ant_query_bt_info(btcoexist);
>> - halbtc8821a1ant_monitor_bt_ctr(btcoexist);
>> - btc8821a1ant_mon_bt_en_dis(btcoexist);
>> -#else
>> - if (halbtc8821a1ant_Is_wifi_status_changed(btcoexist) ||
>> - coex_dm->auto_tdma_adjust) {
>> - if (coex_sta->special_pkt_period_cnt > 2)
>> - halbtc8821a1ant_run_coexist_mechanism(btcoexist);
>> - }
>> -
>> - coex_sta->special_pkt_period_cnt++;
>> -#endif
>> -}
>> diff --git a/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.h
>> b/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.h
>> index 20e9048..c3eab15 100644
>> --- a/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.h
>> +++ b/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.h
>> @@ -168,21 +168,7 @@ struct coex_sta_8821a_1ant {
>> * The following is interface which will notify coex module.
>> *===========================================
>> */
>> -void ex_halbtc8821a1ant_init_hwconfig(struct btc_coexist *btcoexist);
>> -void ex_halbtc8821a1ant_init_coex_dm(struct btc_coexist *btcoexist);
>> -void ex_halbtc8821a1ant_ips_notify(struct btc_coexist *btcoexist, u8
>> type);
>> -void ex_halbtc8821a1ant_lps_notify(struct btc_coexist *btcoexist, u8
>> type);
>> -void ex_halbtc8821a1ant_scan_notify(struct btc_coexist *btcoexist, u8
>> type);
>> -void ex_halbtc8821a1ant_connect_notify(struct btc_coexist *btcoexist, u8
>> type);
>> void ex_halbtc8821a1ant_media_status_notify(struct btc_coexist
>> *btcoexist,
>> u8 type);
>> -void ex_halbtc8821a1ant_special_packet_notify(struct btc_coexist
>> *btcoexist,
>> - u8 type);
>> -void ex_halbtc8821a1ant_bt_info_notify(struct btc_coexist *btcoexist,
>> - u8 *tmpbuf, u8 length);
>> -void ex_halbtc8821a1ant_halt_notify(struct btc_coexist *btcoexist);
>> -void ex_halbtc8821a1ant_pnp_notify(struct btc_coexist *btcoexist, u8
>> pnpstate);
>> -void ex_halbtc8821a1ant_periodical(struct btc_coexist *btcoexist);
>> -void ex_halbtc8821a1ant_display_coex_info(struct btc_coexist *btcoexist);
>> void ex_halbtc8821a1ant_dbg_control(struct btc_coexist *btcoexist, u8
>> op_code,
>> u8 op_len, u8 *data);
>>
Hi
I was absolutely sure that was in the brcm80211/brcmsmac part for some
reason, but I should of course double-checked it!
Apologies, I will not send any more patches.
Kind regards
Rickard Strandqvist
^ permalink raw reply
* Re: [PATCH RESEND 2/2] wlcore: align member-assigns in a structure-copy block
From: Eliad Peller @ 2015-01-11 10:22 UTC (permalink / raw)
To: Kalle Valo
Cc: Giel van Schijndel, LKML, John W. Linville, Arik Nemtsov,
open list:TI WILINK WIRELES..., open list:NETWORKING DRIVERS
In-Reply-To: <87vbkfga32.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>
On Fri, Jan 9, 2015 at 7:03 PM, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> Giel van Schijndel <me-sZ9Uef1cvPWHXe+LvDLADg@public.gmane.org> writes:
>
>> This highlights the differences (e.g. the bug fixed in the previous
>> commit).
>>
>> Signed-off-by: Giel van Schijndel <me-sZ9Uef1cvPWHXe+LvDLADg@public.gmane.org>
>> ---
>> drivers/net/wireless/ti/wlcore/acx.c | 22 +++++++++++-----------
>> 1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c
>> index f28fa3b..93a2fa8 100644
>> --- a/drivers/net/wireless/ti/wlcore/acx.c
>> +++ b/drivers/net/wireless/ti/wlcore/acx.c
>> @@ -1715,17 +1715,17 @@ int wl12xx_acx_config_hangover(struct wl1271 *wl)
>> goto out;
>> }
>>
>> - acx->recover_time = cpu_to_le32(conf->recover_time);
>> - acx->hangover_period = conf->hangover_period;
>> - acx->dynamic_mode = conf->dynamic_mode;
>> - acx->early_termination_mode = conf->early_termination_mode;
>> - acx->max_period = conf->max_period;
>> - acx->min_period = conf->min_period;
>> - acx->increase_delta = conf->increase_delta;
>> - acx->decrease_delta = conf->decrease_delta;
>> - acx->quiet_time = conf->quiet_time;
>> - acx->increase_time = conf->increase_time;
>> - acx->window_size = conf->window_size;
>> + acx->recover_time = cpu_to_le32(conf->recover_time);
>> + acx->hangover_period = conf->hangover_period;
>> + acx->dynamic_mode = conf->dynamic_mode;
>> + acx->early_termination_mode = conf->early_termination_mode;
>> + acx->max_period = conf->max_period;
>> + acx->min_period = conf->min_period;
>> + acx->increase_delta = conf->increase_delta;
>> + acx->decrease_delta = conf->decrease_delta;
>> + acx->quiet_time = conf->quiet_time;
>> + acx->increase_time = conf->increase_time;
>> + acx->window_size = conf->window_size;
>
> I would like to get an ACK from one of the wlcore developers if I should
> apply this (or not).
>
I don't have a strong opinion here.
However, it looks pretty much redundant to take a random blob (which
was just fixed by a correct patch) and re-indent it.
The rest of the file doesn't follow this style, so i don't see a good
reason to apply it here.
I agree such indentation have some benefit, but it won't help with the
more common use case (of copy-paste error) of copying the wrong field
(i.e. D->a = S->b instead of D->a = S->a).
For these cases the macros suggested by Arend and Johannes will do the
trick. However i usually dislike such macros, as they tend to break
some IDE features (e.g. auto completion).
Maybe we can come up with some nice spatch to catch these cases.
Just my 2c.
Eliad.
--
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
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