Netdev List
 help / color / mirror / Atom feed
* [PATCH] net/bonding: bonding: Adjust coding style for bond_3ad files.
From: Rafael Azenha Aquini @ 2011-05-04 22:18 UTC (permalink / raw)
  To: kernel-janitors.vger.kernel.org; +Cc: Jay Vosburgh, Andy Gospodarek, netdev

Howdy,

While I was studying the code under the bond_3ad hood I realized that its coding
style did not follow all Documentation/CodingStyle recommendations. As a tiny
collaboration I did some mods there, in an attempt to make that code stick as
closely as possible with the Kernel coding style.

Signed-off-by: Rafael Aquini <aquini@linux.com>

---
 drivers/net/bonding/bond_3ad.c |  918 +++++++++++++++++++++++-----------------
 drivers/net/bonding/bond_3ad.h |  189 +++++----
 2 files changed, 633 insertions(+), 474 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 31912f1..97e7528 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -34,14 +34,14 @@
 #include "bonding.h"
 #include "bond_3ad.h"
 
-// General definitions
+/* General definitions */
 #define AD_SHORT_TIMEOUT           1
 #define AD_LONG_TIMEOUT            0
 #define AD_STANDBY                 0x2
 #define AD_MAX_TX_IN_SECOND        3
 #define AD_COLLECTOR_MAX_DELAY     0
 
-// Timer definitions(43.4.4 in the 802.3ad standard)
+/* Timer definitions(43.4.4 in the 802.3ad standard) */
 #define AD_FAST_PERIODIC_TIME      1
 #define AD_SLOW_PERIODIC_TIME      30
 #define AD_SHORT_TIMEOUT_TIME      (3*AD_FAST_PERIODIC_TIME)
@@ -49,7 +49,7 @@
 #define AD_CHURN_DETECTION_TIME    60
 #define AD_AGGREGATE_WAIT_TIME     2
 
-// Port state definitions(43.4.2.2 in the 802.3ad standard)
+/* Port state definitions(43.4.2.2 in the 802.3ad standard) */
 #define AD_STATE_LACP_ACTIVITY   0x1
 #define AD_STATE_LACP_TIMEOUT    0x2
 #define AD_STATE_AGGREGATION     0x4
@@ -59,7 +59,10 @@
 #define AD_STATE_DEFAULTED       0x40
 #define AD_STATE_EXPIRED         0x80
 
-// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
+/*
+ * Port Variables definitions used by the State
+ * Machines (43.4.7 in the 802.3ad standard)
+ */
 #define AD_PORT_BEGIN           0x1
 #define AD_PORT_LACP_ENABLED    0x2
 #define AD_PORT_ACTOR_CHURN     0x4
@@ -71,26 +74,25 @@
 #define AD_PORT_SELECTED        0x100
 #define AD_PORT_MOVED           0x200
 
-// Port Key definitions
-// key is determined according to the link speed, duplex and
-// user key(which is yet not supported)
-//              ------------------------------------------------------------
-// Port key :   | User key                       |      Speed       |Duplex|
-//              ------------------------------------------------------------
-//              16                               6               1 0
+/*
+ * Port Key definitions:
+ *  key is determined according to the link speed, duplex and
+ *  user key (which is yet not supported)
+ *             ------------------------------------------------------------
+ *  Port key:  | User key                       |      Speed       |Duplex|
+ *             ------------------------------------------------------------
+ *             16                               6                  1      0
+ */
 #define  AD_DUPLEX_KEY_BITS    0x1
 #define  AD_SPEED_KEY_BITS     0x3E
 #define  AD_USER_KEY_BITS      0xFFC0
 
-//dalloun
 #define     AD_LINK_SPEED_BITMASK_1MBPS       0x1
 #define     AD_LINK_SPEED_BITMASK_10MBPS      0x2
 #define     AD_LINK_SPEED_BITMASK_100MBPS     0x4
 #define     AD_LINK_SPEED_BITMASK_1000MBPS    0x8
 #define     AD_LINK_SPEED_BITMASK_10000MBPS   0x10
-//endalloun
 
-// compare MAC addresses
 #define MAC_ADDRESS_COMPARE(A, B) memcmp(A, B, ETH_ALEN)
 
 static struct mac_addr null_mac_addr = { { 0, 0, 0, 0, 0, 0 } };
@@ -99,7 +101,7 @@ static const int ad_delta_in_ticks = (AD_TIMER_INTERVAL * HZ) / 1000;
 
 static const u8 lacpdu_mcast_addr[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
 
-// ================= main 802.3ad protocol functions ==================
+/* ================= main 802.3ad protocol functions ================== */
 static int ad_lacpdu_send(struct port *port);
 static int ad_marker_send(struct port *port, struct bond_marker *marker);
 static void ad_mux_machine(struct port *port);
@@ -113,14 +115,12 @@ static void ad_initialize_agg(struct aggregator *aggregator);
 static void ad_initialize_port(struct port *port, int lacp_fast);
 static void ad_enable_collecting_distributing(struct port *port);
 static void ad_disable_collecting_distributing(struct port *port);
-static void ad_marker_info_received(struct bond_marker *marker_info, struct port *port);
-static void ad_marker_response_received(struct bond_marker *marker, struct port *port);
-
-
-/////////////////////////////////////////////////////////////////////////////////
-// ================= api to bonding and kernel code ==================
-/////////////////////////////////////////////////////////////////////////////////
+static void ad_marker_info_received(struct bond_marker *marker_info,
+							struct port *port);
+static void ad_marker_response_received(struct bond_marker *marker,
+							struct port *port);
 
+/* ================= api to bonding and kernel code ================== */
 /**
  * __get_bond_by_port - get the port's bonding struct
  * @port: the port we're looking at
@@ -161,7 +161,6 @@ static inline struct port *__get_next_port(struct port *port)
 	struct bonding *bond = __get_bond_by_port(port);
 	struct slave *slave = port->slave;
 
-	// If there's no bond for this port, or this is the last slave
 	if ((bond == NULL) || (slave->next == bond->first_slave))
 		return NULL;
 
@@ -179,7 +178,6 @@ static inline struct aggregator *__get_first_agg(struct port *port)
 {
 	struct bonding *bond = __get_bond_by_port(port);
 
-	// If there's no bond for this port, or bond has no slaves
 	if ((bond == NULL) || (bond->slave_cnt == 0))
 		return NULL;
 
@@ -198,7 +196,6 @@ static inline struct aggregator *__get_next_agg(struct aggregator *aggregator)
 	struct slave *slave = aggregator->slave;
 	struct bonding *bond = bond_get_bond_by_slave(slave);
 
-	// If there's no bond for this aggregator, or this is the last slave
 	if ((bond == NULL) || (slave->next == bond->first_slave))
 		return NULL;
 
@@ -316,10 +313,12 @@ static u16 __get_link_speed(struct port *port)
 	struct slave *slave = port->slave;
 	u16 speed;
 
-	/* this if covers only a special case: when the configuration starts with
-	 * link down, it sets the speed to 0.
-	 * This is done in spite of the fact that the e100 driver reports 0 to be
-	 * compatible with MVT in the future.*/
+	/*
+	 * this if covers only a special case: when the configuration
+	 * starts with link down, it sets the speed to 0.
+	 * This is done in spite of the fact that the e100 driver
+	 * reports 0 to be compatible with MVT in the future.
+	 */
 	if (slave->link != BOND_LINK_UP)
 		speed = 0;
 	else {
@@ -341,13 +340,14 @@ static u16 __get_link_speed(struct port *port)
 			break;
 
 		default:
-			speed = 0; // unknown speed value from ethtool. shouldn't happen
+			/* unknown speed value from ethtool. shouldn't happen */
+			speed = 0;
 			break;
 		}
 	}
 
 	pr_debug("Port %d Received link speed %d update from adapter\n",
-		 port->actor_port_number, speed);
+						port->actor_port_number, speed);
 	return speed;
 }
 
@@ -365,22 +365,26 @@ static u8 __get_duplex(struct port *port)
 
 	u8 retval;
 
-	//  handling a special case: when the configuration starts with
-	// link down, it sets the duplex to 0.
+	/*
+	 * handling a special case: when the configuration starts with
+	 * link down, it sets the duplex to 0.
+	 */
 	if (slave->link != BOND_LINK_UP)
 		retval = 0x0;
 	else {
 		switch (slave->duplex) {
 		case DUPLEX_FULL:
 			retval = 0x1;
-			pr_debug("Port %d Received status full duplex update from adapter\n",
-				 port->actor_port_number);
+			pr_debug("Port %d Received status "
+					"full duplex update from adapter\n",
+					port->actor_port_number);
 			break;
 		case DUPLEX_HALF:
 		default:
 			retval = 0x0;
-			pr_debug("Port %d Received status NOT full duplex update from adapter\n",
-				 port->actor_port_number);
+			pr_debug("Port %d Received status NOT full "
+					"duplex update from adapter\n",
+					port->actor_port_number);
 			break;
 		}
 	}
@@ -394,12 +398,9 @@ static u8 __get_duplex(struct port *port)
  */
 static inline void __initialize_port_locks(struct port *port)
 {
-	// make sure it isn't called twice
 	spin_lock_init(&(SLAVE_AD_INFO(port->slave).state_machine_lock));
 }
 
-//conversions
-
 /**
  * __ad_timer_to_ticks - convert a given timer type to AD module ticks
  * @timer_type:	which timer to operate
@@ -411,36 +412,32 @@ static inline void __initialize_port_locks(struct port *port)
  */
 static u16 __ad_timer_to_ticks(u16 timer_type, u16 par)
 {
-	u16 retval = 0; /* to silence the compiler */
+	u16 retval = 0;
 
 	switch (timer_type) {
-	case AD_CURRENT_WHILE_TIMER:   // for rx machine usage
+	case AD_CURRENT_WHILE_TIMER:	/* for rx machine usage */
 		if (par)
-			retval = (AD_SHORT_TIMEOUT_TIME*ad_ticks_per_sec); // short timeout
+			retval = (AD_SHORT_TIMEOUT_TIME*ad_ticks_per_sec);
 		else
-			retval = (AD_LONG_TIMEOUT_TIME*ad_ticks_per_sec); // long timeout
+			retval = (AD_LONG_TIMEOUT_TIME*ad_ticks_per_sec);
 		break;
-	case AD_ACTOR_CHURN_TIMER:	    // for local churn machine
+	case AD_ACTOR_CHURN_TIMER:	/* for local churn machine */
 		retval = (AD_CHURN_DETECTION_TIME*ad_ticks_per_sec);
 		break;
-	case AD_PERIODIC_TIMER:	    // for periodic machine
-		retval = (par*ad_ticks_per_sec); // long timeout
+	case AD_PERIODIC_TIMER:		/* for periodic machine */
+		retval = (par*ad_ticks_per_sec);
 		break;
-	case AD_PARTNER_CHURN_TIMER:   // for remote churn machine
+	case AD_PARTNER_CHURN_TIMER:	/* for remote churn machine */
 		retval = (AD_CHURN_DETECTION_TIME*ad_ticks_per_sec);
 		break;
-	case AD_WAIT_WHILE_TIMER:	    // for selection machine
+	case AD_WAIT_WHILE_TIMER:	/* for selection machine */
 		retval = (AD_AGGREGATE_WAIT_TIME*ad_ticks_per_sec);
 		break;
 	}
 	return retval;
 }
 
-
-/////////////////////////////////////////////////////////////////////////////////
-// ================= ad_rx_machine helper functions ==================
-/////////////////////////////////////////////////////////////////////////////////
-
+/* ================= ad_rx_machine helper functions ================== */
 /**
  * __choose_matched - update a port's matched variable from a received lacpdu
  * @lacpdu: the lacpdu we've received
@@ -466,17 +463,17 @@ static u16 __ad_timer_to_ticks(u16 timer_type, u16 par)
  */
 static void __choose_matched(struct lacpdu *lacpdu, struct port *port)
 {
-	// check if all parameters are alike
 	if (((ntohs(lacpdu->partner_port) == port->actor_port_number) &&
-	     (ntohs(lacpdu->partner_port_priority) == port->actor_port_priority) &&
-	     !MAC_ADDRESS_COMPARE(&(lacpdu->partner_system), &(port->actor_system)) &&
-	     (ntohs(lacpdu->partner_system_priority) == port->actor_system_priority) &&
-	     (ntohs(lacpdu->partner_key) == port->actor_oper_port_key) &&
-	     ((lacpdu->partner_state & AD_STATE_AGGREGATION) == (port->actor_oper_port_state & AD_STATE_AGGREGATION))) ||
-	    // or this is individual link(aggregation == FALSE)
-	    ((lacpdu->actor_state & AD_STATE_AGGREGATION) == 0)
-		) {
-		// update the state machine Matched variable
+	     (ntohs(lacpdu->partner_port_priority) == port->actor_port_priority)
+	    && !MAC_ADDRESS_COMPARE(&(lacpdu->partner_system),
+							&(port->actor_system))
+	    && (ntohs(lacpdu->partner_system_priority) ==
+						   port->actor_system_priority)
+	    && (ntohs(lacpdu->partner_key) == port->actor_oper_port_key)
+	    && ((lacpdu->partner_state & AD_STATE_AGGREGATION) ==
+			(port->actor_oper_port_state & AD_STATE_AGGREGATION)))
+	    || ((lacpdu->actor_state & AD_STATE_AGGREGATION) == 0)) {
+		/* update the state machine Selected variable */
 		port->sm_vars |= AD_PORT_MATCHED;
 	} else {
 		port->sm_vars &= ~AD_PORT_MATCHED;
@@ -498,7 +495,7 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 		struct port_params *partner = &port->partner_oper;
 
 		__choose_matched(lacpdu, port);
-		// record the new parameter values for the partner operational
+		/* record the new values for the operational partner */
 		partner->port_number = ntohs(lacpdu->actor_port);
 		partner->port_priority = ntohs(lacpdu->actor_port_priority);
 		partner->system = lacpdu->actor_system;
@@ -506,10 +503,12 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 		partner->key = ntohs(lacpdu->actor_key);
 		partner->port_state = lacpdu->actor_state;
 
-		// set actor_oper_port_state.defaulted to FALSE
+		/* set actor_oper_port_state.defaulted to FALSE */
 		port->actor_oper_port_state &= ~AD_STATE_DEFAULTED;
 
-		// set the partner sync. to on if the partner is sync. and the port is matched
+		/* switch on partner sync. if partner is synchronized,
+		 * and the port is matched
+		 */
 		if ((port->sm_vars & AD_PORT_MATCHED)
 		    && (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION))
 			partner->port_state |= AD_STATE_SYNCHRONIZATION;
@@ -529,11 +528,8 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 static void __record_default(struct port *port)
 {
 	if (port) {
-		// record the partner admin parameters
 		memcpy(&port->partner_oper, &port->partner_admin,
 		       sizeof(struct port_params));
-
-		// set actor_oper_port_state.defaulted to true
 		port->actor_oper_port_state |= AD_STATE_DEFAULTED;
 	}
 }
@@ -556,14 +552,17 @@ static void __update_selected(struct lacpdu *lacpdu, struct port *port)
 	if (lacpdu && port) {
 		const struct port_params *partner = &port->partner_oper;
 
-		// check if any parameter is different
-		if (ntohs(lacpdu->actor_port) != partner->port_number ||
-		    ntohs(lacpdu->actor_port_priority) != partner->port_priority ||
-		    MAC_ADDRESS_COMPARE(&lacpdu->actor_system, &partner->system) ||
-		    ntohs(lacpdu->actor_system_priority) != partner->system_priority ||
-		    ntohs(lacpdu->actor_key) != partner->key ||
-		    (lacpdu->actor_state & AD_STATE_AGGREGATION) != (partner->port_state & AD_STATE_AGGREGATION)) {
-			// update the state machine Selected variable
+		if (ntohs(lacpdu->actor_port) != partner->port_number
+		    || ntohs(lacpdu->actor_port_priority) !=
+							partner->port_priority
+		    || MAC_ADDRESS_COMPARE(&lacpdu->actor_system,
+							&partner->system)
+		    || ntohs(lacpdu->actor_system_priority) !=
+							partner->system_priority
+		    || ntohs(lacpdu->actor_key) != partner->key
+		    || (lacpdu->actor_state & AD_STATE_AGGREGATION) !=
+		    (partner->port_state & AD_STATE_AGGREGATION)) {
+			/* update the state machine Selected variable */
 			port->sm_vars &= ~AD_PORT_SELECTED;
 		}
 	}
@@ -587,7 +586,6 @@ static void __update_default_selected(struct port *port)
 		const struct port_params *admin = &port->partner_admin;
 		const struct port_params *oper = &port->partner_oper;
 
-		// check if any parameter is different
 		if (admin->port_number != oper->port_number ||
 		    admin->port_priority != oper->port_priority ||
 		    MAC_ADDRESS_COMPARE(&admin->system, &oper->system) ||
@@ -595,7 +593,7 @@ static void __update_default_selected(struct port *port)
 		    admin->key != oper->key ||
 		    (admin->port_state & AD_STATE_AGGREGATION)
 			!= (oper->port_state & AD_STATE_AGGREGATION)) {
-			// update the state machine Selected variable
+			/* update the state machine Selected variable */
 			port->sm_vars &= ~AD_PORT_SELECTED;
 		}
 	}
@@ -615,20 +613,24 @@ static void __update_default_selected(struct port *port)
  */
 static void __update_ntt(struct lacpdu *lacpdu, struct port *port)
 {
-	// validate lacpdu and port
 	if (lacpdu && port) {
-		// check if any parameter is different
-		if ((ntohs(lacpdu->partner_port) != port->actor_port_number) ||
-		    (ntohs(lacpdu->partner_port_priority) != port->actor_port_priority) ||
-		    MAC_ADDRESS_COMPARE(&(lacpdu->partner_system), &(port->actor_system)) ||
-		    (ntohs(lacpdu->partner_system_priority) != port->actor_system_priority) ||
-		    (ntohs(lacpdu->partner_key) != port->actor_oper_port_key) ||
-		    ((lacpdu->partner_state & AD_STATE_LACP_ACTIVITY) != (port->actor_oper_port_state & AD_STATE_LACP_ACTIVITY)) ||
-		    ((lacpdu->partner_state & AD_STATE_LACP_TIMEOUT) != (port->actor_oper_port_state & AD_STATE_LACP_TIMEOUT)) ||
-		    ((lacpdu->partner_state & AD_STATE_SYNCHRONIZATION) != (port->actor_oper_port_state & AD_STATE_SYNCHRONIZATION)) ||
-		    ((lacpdu->partner_state & AD_STATE_AGGREGATION) != (port->actor_oper_port_state & AD_STATE_AGGREGATION))
-		   ) {
-
+		if ((ntohs(lacpdu->partner_port) != port->actor_port_number)
+		    || (ntohs(lacpdu->partner_port_priority) !=
+						port->actor_port_priority)
+		    || MAC_ADDRESS_COMPARE(&(lacpdu->partner_system),
+						&(port->actor_system))
+		    || (ntohs(lacpdu->partner_system_priority) !=
+						port->actor_system_priority)
+		    || (ntohs(lacpdu->partner_key) != port->actor_oper_port_key)
+		    || ((lacpdu->partner_state & AD_STATE_LACP_ACTIVITY) !=
+			(port->actor_oper_port_state & AD_STATE_LACP_ACTIVITY))
+		    || ((lacpdu->partner_state & AD_STATE_LACP_TIMEOUT) !=
+			(port->actor_oper_port_state & AD_STATE_LACP_TIMEOUT))
+		    || ((lacpdu->partner_state & AD_STATE_SYNCHRONIZATION) !=
+		       (port->actor_oper_port_state & AD_STATE_SYNCHRONIZATION))
+		    || ((lacpdu->partner_state & AD_STATE_AGGREGATION) !=
+		    (port->actor_oper_port_state & AD_STATE_AGGREGATION))) {
+			/* set port ntt */
 			port->ntt = true;
 		}
 	}
@@ -644,9 +646,7 @@ static void __update_ntt(struct lacpdu *lacpdu, struct port *port)
  */
 static void __attach_bond_to_agg(struct port *port)
 {
-	port = NULL; /* just to satisfy the compiler */
-	// This function does nothing since the parser/multiplexer of the receive
-	// and the parser/multiplexer of the aggregator are already combined
+	port = NULL;
 }
 
 /**
@@ -659,9 +659,7 @@ static void __attach_bond_to_agg(struct port *port)
  */
 static void __detach_bond_from_agg(struct port *port)
 {
-	port = NULL; /* just to satisfy the compiler */
-	// This function does nothing sience the parser/multiplexer of the receive
-	// and the parser/multiplexer of the aggregator are already combined
+	port = NULL;
 }
 
 /**
@@ -675,7 +673,9 @@ static int __agg_ports_are_ready(struct aggregator *aggregator)
 	int retval = 1;
 
 	if (aggregator) {
-		// scan all ports in this aggregator to verfy if they are all ready
+		/* scan all ports in this aggregator to verfy
+		 * if they are all ready
+		 */
 		for (port = aggregator->lag_ports;
 		     port;
 		     port = port->next_port_in_aggregator) {
@@ -737,7 +737,7 @@ static u32 __get_agg_bandwidth(struct aggregator *aggregator)
 			bandwidth = aggregator->num_of_ports * 10000;
 			break;
 		default:
-			bandwidth = 0; /*to silence the compiler ....*/
+			bandwidth = 0;
 		}
 	}
 	return bandwidth;
@@ -809,10 +809,7 @@ static inline void __update_lacpdu_from_port(struct port *port)
 	 */
 }
 
-//////////////////////////////////////////////////////////////////////////////////////
-// ================= main 802.3ad protocol code ======================================
-//////////////////////////////////////////////////////////////////////////////////////
-
+/* ================= main 802.3ad protocol code ================= */
 /**
  * ad_lacpdu_send - send out a lacpdu packet on a given port
  * @port: the port we're looking at
@@ -845,7 +842,7 @@ static int ad_lacpdu_send(struct port *port)
 	memcpy(lacpdu_header->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
 	lacpdu_header->hdr.h_proto = PKT_TYPE_LACPDU;
 
-	lacpdu_header->lacpdu = port->lacpdu; // struct copy
+	lacpdu_header->lacpdu = port->lacpdu;
 
 	dev_queue_xmit(skb);
 
@@ -886,7 +883,7 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
 	memcpy(marker_header->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
 	marker_header->hdr.h_proto = PKT_TYPE_LACPDU;
 
-	marker_header->marker = *marker; // struct copy
+	marker_header->marker = *marker;
 
 	dev_queue_xmit(skb);
 
@@ -902,80 +899,100 @@ static void ad_mux_machine(struct port *port)
 {
 	mux_states_t last_state;
 
-	// keep current State Machine state to compare later if it was changed
 	last_state = port->sm_mux_state;
 
 	if (port->sm_vars & AD_PORT_BEGIN) {
-		port->sm_mux_state = AD_MUX_DETACHED;		 // next state
+		port->sm_mux_state = AD_MUX_DETACHED;
 	} else {
 		switch (port->sm_mux_state) {
 		case AD_MUX_DETACHED:
 			if ((port->sm_vars & AD_PORT_SELECTED)
 			    || (port->sm_vars & AD_PORT_STANDBY))
 				/* if SELECTED or STANDBY */
-				port->sm_mux_state = AD_MUX_WAITING; // next state
+				port->sm_mux_state = AD_MUX_WAITING;
 			break;
 		case AD_MUX_WAITING:
-			// if SELECTED == FALSE return to DETACH state
-			if (!(port->sm_vars & AD_PORT_SELECTED)) { // if UNSELECTED
+			/* if SELECTED == FALSE return to DETACH state */
+			if (!(port->sm_vars & AD_PORT_SELECTED)) {
 				port->sm_vars &= ~AD_PORT_READY_N;
-				// in order to withhold the Selection Logic to check all ports READY_N value
-				// every callback cycle to update ready variable, we check READY_N and update READY here
-				__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
-				port->sm_mux_state = AD_MUX_DETACHED;	 // next state
+				/* in order to withhold the Selection Logic
+				 * to check all ports READY_N value at every
+				 * callback cycle to update ready variable,
+				 * we check READY_N and update READY here
+				 */
+				__set_agg_ports_ready(port->aggregator,
+				       __agg_ports_are_ready(port->aggregator));
+				port->sm_mux_state = AD_MUX_DETACHED;
 				break;
 			}
 
-			// check if the wait_while_timer expired
+			/* check if the wait_while_timer expired */
 			if (port->sm_mux_timer_counter
 			    && !(--port->sm_mux_timer_counter))
 				port->sm_vars |= AD_PORT_READY_N;
 
-			// in order to withhold the selection logic to check all ports READY_N value
-			// every callback cycle to update ready variable, we check READY_N and update READY here
-			__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
-
-			// if the wait_while_timer expired, and the port is in READY state, move to ATTACHED state
+			/* in order to withhold the selection logic
+			 * to check all ports READY_N value at every
+			 * callback cycle to update ready variable,
+			 * we check READY_N and update READY here
+			 */
+			__set_agg_ports_ready(port->aggregator,
+				       __agg_ports_are_ready(port->aggregator));
+
+			/* if the wait_while_timer expired,  and the port
+			 * is in READY state, move to ATTACHED state
+			 */
 			if ((port->sm_vars & AD_PORT_READY)
 			    && !port->sm_mux_timer_counter)
-				port->sm_mux_state = AD_MUX_ATTACHED;	 // next state
+				port->sm_mux_state = AD_MUX_ATTACHED;
 			break;
 		case AD_MUX_ATTACHED:
-			// check also if agg_select_timer expired(so the edable port will take place only after this timer)
-			if ((port->sm_vars & AD_PORT_SELECTED) && (port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) && !__check_agg_selection_timer(port)) {
-				port->sm_mux_state = AD_MUX_COLLECTING_DISTRIBUTING;// next state
-			} else if (!(port->sm_vars & AD_PORT_SELECTED) || (port->sm_vars & AD_PORT_STANDBY)) {	  // if UNSELECTED or STANDBY
+			/* check also if agg_select_timer expired (so the
+			 * edable port will take place only after this timer)
+			 */
+			if ((port->sm_vars & AD_PORT_SELECTED) &&
+			    (port->partner_oper.port_state &
+			     AD_STATE_SYNCHRONIZATION) &&
+			    !__check_agg_selection_timer(port)) {
+				port->sm_mux_state =
+					AD_MUX_COLLECTING_DISTRIBUTING;
+			} else if (!(port->sm_vars & AD_PORT_SELECTED) ||
+				   (port->sm_vars & AD_PORT_STANDBY)) {
+				/* if UNSELECTED or STANDBY */
 				port->sm_vars &= ~AD_PORT_READY_N;
-				// in order to withhold the selection logic to check all ports READY_N value
-				// every callback cycle to update ready variable, we check READY_N and update READY here
-				__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
-				port->sm_mux_state = AD_MUX_DETACHED;// next state
+				/* in order to withhold the Selection Logic
+				 * to check all ports READY_N value at every
+				 * callback cycle to update ready variable,
+				 * we check READY_N and update READY here
+				 */
+				__set_agg_ports_ready(port->aggregator,
+				       __agg_ports_are_ready(port->aggregator));
+				port->sm_mux_state = AD_MUX_DETACHED;
 			}
 			break;
 		case AD_MUX_COLLECTING_DISTRIBUTING:
-			if (!(port->sm_vars & AD_PORT_SELECTED) || (port->sm_vars & AD_PORT_STANDBY) ||
-			    !(port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION)
-			   ) {
-				port->sm_mux_state = AD_MUX_ATTACHED;// next state
-
+			if (!(port->sm_vars & AD_PORT_SELECTED) ||
+			    (port->sm_vars & AD_PORT_STANDBY) ||
+			    !(port->partner_oper.port_state &
+			     AD_STATE_SYNCHRONIZATION)) {
+				port->sm_mux_state = AD_MUX_ATTACHED;
 			} else {
-				// if port state hasn't changed make
-				// sure that a collecting distributing
-				// port in an active aggregator is enabled
+				/* if port state hasn't changed make
+				 * sure that a collecting distributing
+				 * port in an active aggregator is enabled
+				 */
 				if (port->aggregator &&
 				    port->aggregator->is_active &&
-				    !__port_is_enabled(port)) {
-
+				    !__port_is_enabled(port))
 					__enable_port(port);
-				}
 			}
 			break;
-		default:    //to silence the compiler
+		default:
 			break;
 		}
 	}
 
-	// check if the state machine was changed
+	/* check if the state machine was changed */
 	if (port->sm_mux_state != last_state) {
 		pr_debug("Mux Machine: Port=%d, Last State=%d, Curr State=%d\n",
 			 port->actor_port_number, last_state,
@@ -983,14 +1000,16 @@ static void ad_mux_machine(struct port *port)
 		switch (port->sm_mux_state) {
 		case AD_MUX_DETACHED:
 			__detach_bond_from_agg(port);
-			port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
+			port->actor_oper_port_state &=
+					~AD_STATE_SYNCHRONIZATION;
 			ad_disable_collecting_distributing(port);
 			port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
 			port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
 			port->ntt = true;
 			break;
 		case AD_MUX_WAITING:
-			port->sm_mux_timer_counter = __ad_timer_to_ticks(AD_WAIT_WHILE_TIMER, 0);
+			port->sm_mux_timer_counter =
+				__ad_timer_to_ticks(AD_WAIT_WHILE_TIMER, 0);
 			break;
 		case AD_MUX_ATTACHED:
 			__attach_bond_to_agg(port);
@@ -1006,7 +1025,7 @@ static void ad_mux_machine(struct port *port)
 			ad_enable_collecting_distributing(port);
 			port->ntt = true;
 			break;
-		default:    //to silence the compiler
+		default:
 			break;
 		}
 	}
@@ -1025,59 +1044,61 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 {
 	rx_states_t last_state;
 
-	// keep current State Machine state to compare later if it was changed
 	last_state = port->sm_rx_state;
 
-	// check if state machine should change state
-	// first, check if port was reinitialized
+	/* check if state machine should change state
+	 first, check if port was reinitialized */
 	if (port->sm_vars & AD_PORT_BEGIN)
 		/* next state */
 		port->sm_rx_state = AD_RX_INITIALIZE;
-	// check if port is not enabled
+	/* check if port is not enabled */
 	else if (!(port->sm_vars & AD_PORT_BEGIN)
 		 && !port->is_enabled && !(port->sm_vars & AD_PORT_MOVED))
 		/* next state */
 		port->sm_rx_state = AD_RX_PORT_DISABLED;
-	// check if new lacpdu arrived
-	else if (lacpdu && ((port->sm_rx_state == AD_RX_EXPIRED) || (port->sm_rx_state == AD_RX_DEFAULTED) || (port->sm_rx_state == AD_RX_CURRENT))) {
-		port->sm_rx_timer_counter = 0; // zero timer
+	/* check if new lacpdu arrived */
+	else if (lacpdu && ((port->sm_rx_state == AD_RX_EXPIRED) ||
+		 (port->sm_rx_state == AD_RX_DEFAULTED) ||
+		 (port->sm_rx_state == AD_RX_CURRENT))) {
+		port->sm_rx_timer_counter = 0; /* zero timer */
 		port->sm_rx_state = AD_RX_CURRENT;
 	} else {
-		// if timer is on, and if it is expired
-		if (port->sm_rx_timer_counter && !(--port->sm_rx_timer_counter)) {
+		/* if timer is on, and if it is expired */
+		if (port->sm_rx_timer_counter &&
+		    !(--port->sm_rx_timer_counter)) {
 			switch (port->sm_rx_state) {
 			case AD_RX_EXPIRED:
-				port->sm_rx_state = AD_RX_DEFAULTED;		// next state
+				port->sm_rx_state = AD_RX_DEFAULTED;
 				break;
 			case AD_RX_CURRENT:
-				port->sm_rx_state = AD_RX_EXPIRED;	    // next state
+				port->sm_rx_state = AD_RX_EXPIRED;
 				break;
-			default:    //to silence the compiler
+			default:
 				break;
 			}
 		} else {
-			// if no lacpdu arrived and no timer is on
+			/* if no lacpdu arrived and no timer is on */
 			switch (port->sm_rx_state) {
 			case AD_RX_PORT_DISABLED:
 				if (port->sm_vars & AD_PORT_MOVED)
-					port->sm_rx_state = AD_RX_INITIALIZE;	    // next state
+					port->sm_rx_state = AD_RX_INITIALIZE;
 				else if (port->is_enabled
 					 && (port->sm_vars
 					     & AD_PORT_LACP_ENABLED))
-					port->sm_rx_state = AD_RX_EXPIRED;	// next state
+					port->sm_rx_state = AD_RX_EXPIRED;
 				else if (port->is_enabled
 					 && ((port->sm_vars
 					      & AD_PORT_LACP_ENABLED) == 0))
-					port->sm_rx_state = AD_RX_LACP_DISABLED;    // next state
+					port->sm_rx_state = AD_RX_LACP_DISABLED;
 				break;
-			default:    //to silence the compiler
+			default:
 				break;
 
 			}
 		}
 	}
 
-	// check if the State machine was changed or new lacpdu arrived
+	/* check if the State machine was changed or new lacpdu arrived */
 	if ((port->sm_rx_state != last_state) || (lacpdu)) {
 		pr_debug("Rx Machine: Port=%d, Last State=%d, Curr State=%d\n",
 			 port->actor_port_number, last_state,
@@ -1092,7 +1113,7 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			__record_default(port);
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
 			port->sm_vars &= ~AD_PORT_MOVED;
-			port->sm_rx_state = AD_RX_PORT_DISABLED;	// next state
+			port->sm_rx_state = AD_RX_PORT_DISABLED;
 
 			/*- Fall Through -*/
 
@@ -1107,14 +1128,20 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
 			break;
 		case AD_RX_EXPIRED:
-			//Reset of the Synchronization flag. (Standard 43.4.12)
-			//This reset cause to disable this port in the COLLECTING_DISTRIBUTING state of the
-			//mux machine in case of EXPIRED even if LINK_DOWN didn't arrive for the port.
-			port->partner_oper.port_state &= ~AD_STATE_SYNCHRONIZATION;
+			/* Reset of the Synchronization flag. (Standard 43.4.12)
+			 * This reset cause to disable this port in the
+			 * COLLECTING_DISTRIBUTING state of the mux machine
+			 * in case of EXPIRED even if LINK_DOWN didn't arrive
+			 * for the port.
+			 */
+			port->partner_oper.port_state &=
+						~AD_STATE_SYNCHRONIZATION;
 			port->sm_vars &= ~AD_PORT_MATCHED;
 			port->partner_oper.port_state |=
 				AD_STATE_LACP_ACTIVITY;
-			port->sm_rx_timer_counter = __ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER, (u16)(AD_SHORT_TIMEOUT));
+			port->sm_rx_timer_counter =
+				__ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER,
+						(u16)(AD_SHORT_TIMEOUT));
 			port->actor_oper_port_state |= AD_STATE_EXPIRED;
 			break;
 		case AD_RX_DEFAULTED:
@@ -1124,28 +1151,38 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
 			break;
 		case AD_RX_CURRENT:
-			// detect loopback situation
-			if (!MAC_ADDRESS_COMPARE(&(lacpdu->actor_system), &(port->actor_system))) {
-				// INFO_RECEIVED_LOOPBACK_FRAMES
-				pr_err("%s: An illegal loopback occurred on adapter (%s).\n"
-				       "Check the configuration to verify that all adapters are connected to 802.3ad compliant switch ports\n",
-				       port->slave->dev->master->name, port->slave->dev->name);
+			/* detect loopback situation */
+			if (!MAC_ADDRESS_COMPARE(&(lacpdu->actor_system),
+			    &(port->actor_system))) {
+				/* INFO_RECEIVED_LOOPBACK_FRAMES */
+				pr_err("%s: An illegal loopback occurred on "
+					"adapter (%s).\nCheck the configuration"
+					" to verify that all adapters are "
+					"connected to 802.3ad compliant "
+					"switch ports\n",
+						port->slave->dev->master->name,
+						port->slave->dev->name);
 				return;
 			}
 			__update_selected(lacpdu, port);
 			__update_ntt(lacpdu, port);
 			__record_pdu(lacpdu, port);
-			port->sm_rx_timer_counter = __ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER, (u16)(port->actor_oper_port_state & AD_STATE_LACP_TIMEOUT));
+			port->sm_rx_timer_counter =
+				__ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER,
+					(u16)(port->actor_oper_port_state
+					& AD_STATE_LACP_TIMEOUT));
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
-			// verify that if the aggregator is enabled, the port is enabled too.
-			//(because if the link goes down for a short time, the 802.3ad will not
-			// catch it, and the port will continue to be disabled)
+			/* verify that if the aggregator is enabled,
+			 * the port is enabled too. (because if the link
+			 * goes down for a short time, the 802.3ad will not
+			 *  catch it, and the port will continue to be disabled)
+			 */
 			if (port->aggregator
 			    && port->aggregator->is_active
 			    && !__port_is_enabled(port))
 				__enable_port(port);
 			break;
-		default:    //to silence the compiler
+		default:
 			break;
 		}
 	}
@@ -1158,9 +1195,11 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
  */
 static void ad_tx_machine(struct port *port)
 {
-	// check if tx timer expired, to verify that we do not send more than 3 packets per second
+	/* check if tx timer expired, to verify that we do not
+	 * send more than 3 packets per second
+	 */
 	if (port->sm_tx_timer_counter && !(--port->sm_tx_timer_counter)) {
-		// check if there is something to send
+		/* check if there is something to send */
 		if (port->ntt && (port->sm_vars & AD_PORT_LACP_ENABLED)) {
 			__update_lacpdu_from_port(port);
 
@@ -1168,14 +1207,17 @@ static void ad_tx_machine(struct port *port)
 				pr_debug("Sent LACPDU on port %d\n",
 					 port->actor_port_number);
 
-				/* mark ntt as false, so it will not be sent again until
-				   demanded */
+				/* mark ntt as false, so it will not be
+				 * sent again until demanded
+				 */
 				port->ntt = false;
 			}
 		}
-		// restart tx timer(to verify that we will not exceed AD_MAX_TX_IN_SECOND
+		/* restart tx timer(to verify that we will not
+		 * exceed AD_MAX_TX_IN_SECOND
+		 */
 		port->sm_tx_timer_counter =
-			ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
+				ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
 	}
 }
 
@@ -1189,76 +1231,92 @@ static void ad_periodic_machine(struct port *port)
 {
 	periodic_states_t last_state;
 
-	// keep current state machine state to compare later if it was changed
+	/* keep current state machine state to compare
+	 * later if it was changed
+	 */
 	last_state = port->sm_periodic_state;
 
-	// check if port was reinitialized
-	if (((port->sm_vars & AD_PORT_BEGIN) || !(port->sm_vars & AD_PORT_LACP_ENABLED) || !port->is_enabled) ||
-	    (!(port->actor_oper_port_state & AD_STATE_LACP_ACTIVITY) && !(port->partner_oper.port_state & AD_STATE_LACP_ACTIVITY))
-	   ) {
-		port->sm_periodic_state = AD_NO_PERIODIC;	     // next state
+	/* check if port was reinitialized */
+	if (((port->sm_vars & AD_PORT_BEGIN) ||
+	    !(port->sm_vars & AD_PORT_LACP_ENABLED) ||
+	    !port->is_enabled) ||
+	    (!(port->actor_oper_port_state & AD_STATE_LACP_ACTIVITY) &&
+	     !(port->partner_oper.port_state & AD_STATE_LACP_ACTIVITY))) {
+		port->sm_periodic_state = AD_NO_PERIODIC; /* next state */
 	}
-	// check if state machine should change state
+	/* check if state machine should change state */
 	else if (port->sm_periodic_timer_counter) {
-		// check if periodic state machine expired
+		/* check if periodic state machine expired */
 		if (!(--port->sm_periodic_timer_counter)) {
-			// if expired then do tx
-			port->sm_periodic_state = AD_PERIODIC_TX;    // next state
+			/* if expired then do tx, next state */
+			port->sm_periodic_state = AD_PERIODIC_TX;
 		} else {
-			// If not expired, check if there is some new timeout parameter from the partner state
+			/* If not expired, check if there is some
+			 * new timeout parameter from the partner state
+			 */
 			switch (port->sm_periodic_state) {
 			case AD_FAST_PERIODIC:
 				if (!(port->partner_oper.port_state
 				      & AD_STATE_LACP_TIMEOUT))
-					port->sm_periodic_state = AD_SLOW_PERIODIC;  // next state
+					port->sm_periodic_state =
+							AD_SLOW_PERIODIC;
 				break;
 			case AD_SLOW_PERIODIC:
-				if ((port->partner_oper.port_state & AD_STATE_LACP_TIMEOUT)) {
-					// stop current timer
+				if ((port->partner_oper.port_state &
+				     AD_STATE_LACP_TIMEOUT)) {
+					/* stop current timer */
 					port->sm_periodic_timer_counter = 0;
-					port->sm_periodic_state = AD_PERIODIC_TX;	 // next state
+					port->sm_periodic_state =
+							 AD_PERIODIC_TX;
 				}
 				break;
-			default:    //to silence the compiler
+			default:
 				break;
 			}
 		}
 	} else {
 		switch (port->sm_periodic_state) {
 		case AD_NO_PERIODIC:
-			port->sm_periodic_state = AD_FAST_PERIODIC;	 // next state
+			port->sm_periodic_state = AD_FAST_PERIODIC;
 			break;
 		case AD_PERIODIC_TX:
 			if (!(port->partner_oper.port_state
 			      & AD_STATE_LACP_TIMEOUT))
-				port->sm_periodic_state = AD_SLOW_PERIODIC;  // next state
+				port->sm_periodic_state = AD_SLOW_PERIODIC;
 			else
-				port->sm_periodic_state = AD_FAST_PERIODIC;  // next state
+				port->sm_periodic_state = AD_FAST_PERIODIC;
 			break;
-		default:    //to silence the compiler
+		default:
 			break;
 		}
 	}
 
-	// check if the state machine was changed
+	/* check if the state machine was changed */
 	if (port->sm_periodic_state != last_state) {
-		pr_debug("Periodic Machine: Port=%d, Last State=%d, Curr State=%d\n",
+		pr_debug("Periodic Machine: Port=%d, "
+			 "Last State=%d, Curr State=%d\n",
 			 port->actor_port_number, last_state,
 			 port->sm_periodic_state);
 		switch (port->sm_periodic_state) {
 		case AD_NO_PERIODIC:
-			port->sm_periodic_timer_counter = 0;	   // zero timer
+			port->sm_periodic_timer_counter = 0;
 			break;
 		case AD_FAST_PERIODIC:
-			port->sm_periodic_timer_counter = __ad_timer_to_ticks(AD_PERIODIC_TIMER, (u16)(AD_FAST_PERIODIC_TIME))-1; // decrement 1 tick we lost in the PERIODIC_TX cycle
+			/* decrement 1 tick we lost in PERIODIC_TX cycle */
+			port->sm_periodic_timer_counter =
+					__ad_timer_to_ticks(AD_PERIODIC_TIMER,
+						(u16)(AD_FAST_PERIODIC_TIME))-1;
 			break;
 		case AD_SLOW_PERIODIC:
-			port->sm_periodic_timer_counter = __ad_timer_to_ticks(AD_PERIODIC_TIMER, (u16)(AD_SLOW_PERIODIC_TIME))-1; // decrement 1 tick we lost in the PERIODIC_TX cycle
+			/* decrement 1 tick we lost in PERIODIC_TX cycle */
+			port->sm_periodic_timer_counter =
+					__ad_timer_to_ticks(AD_PERIODIC_TIMER,
+						(u16)(AD_SLOW_PERIODIC_TIME))-1;
 			break;
 		case AD_PERIODIC_TX:
 			port->ntt = true;
 			break;
-		default:    //to silence the compiler
+		default:
 			break;
 		}
 	}
@@ -1274,32 +1332,38 @@ static void ad_periodic_machine(struct port *port)
  */
 static void ad_port_selection_logic(struct port *port)
 {
-	struct aggregator *aggregator, *free_aggregator = NULL, *temp_aggregator;
+	struct aggregator *aggregator, *temp_aggregator;
+	struct aggregator *free_aggregator = NULL;
 	struct port *last_port = NULL, *curr_port;
 	int found = 0;
 
-	// if the port is already Selected, do nothing
+	/* if the port is already Selected, do nothing */
 	if (port->sm_vars & AD_PORT_SELECTED)
 		return;
 
-	// if the port is connected to other aggregator, detach it
+	/* if the port is connected to other aggregator, detach it */
 	if (port->aggregator) {
-		// detach the port from its former aggregator
+		/* detach the port from its former aggregator */
 		temp_aggregator = port->aggregator;
 		for (curr_port = temp_aggregator->lag_ports; curr_port;
 		     last_port = curr_port,
 			     curr_port = curr_port->next_port_in_aggregator) {
 			if (curr_port == port) {
 				temp_aggregator->num_of_ports--;
-				if (!last_port) {// if it is the first port attached to the aggregator
+				if (!last_port) {
+					/* if it is the first port attached
+					   to the aggregator */
 					temp_aggregator->lag_ports =
 						port->next_port_in_aggregator;
-				} else {// not the first port attached to the aggregator
+				} else {
+					/* not the first port attached
+					   to the aggregator */
 					last_port->next_port_in_aggregator =
 						port->next_port_in_aggregator;
 				}
 
-				// clear the port's relations to this aggregator
+				/* clear the port's relations
+				   to this aggregator */
 				port->aggregator = NULL;
 				port->next_port_in_aggregator = NULL;
 				port->actor_port_aggregator_identifier = 0;
@@ -1307,41 +1371,51 @@ static void ad_port_selection_logic(struct port *port)
 				pr_debug("Port %d left LAG %d\n",
 					 port->actor_port_number,
 					 temp_aggregator->aggregator_identifier);
-				// if the aggregator is empty, clear its parameters, and set it ready to be attached
+				/* if the aggregator is empty,
+				 * clear its parameters, and set it
+				 * ready to be attached
+				 */
 				if (!temp_aggregator->lag_ports)
 					ad_clear_agg(temp_aggregator);
 				break;
 			}
 		}
-		if (!curr_port) { // meaning: the port was related to an aggregator but was not on the aggregator port list
-			pr_warning("%s: Warning: Port %d (on %s) was related to aggregator %d but was not on its port list\n",
-				   port->slave->dev->master->name,
-				   port->actor_port_number,
-				   port->slave->dev->name,
-				   port->aggregator->aggregator_identifier);
+		if (!curr_port) {
+			/* meaning: the port was related to an aggregator
+			 * but was not on the aggregator port list.
+			 */
+			pr_warning("%s: Warning: Port %d (on %s) was related "
+				   "to aggregator %d but was not on its port "
+				   "list\n", port->slave->dev->master->name,
+				    port->actor_port_number,
+				    port->slave->dev->name,
+				    port->aggregator->aggregator_identifier);
 		}
 	}
-	// search on all aggregators for a suitable aggregator for this port
+
+	/* search on all aggregators for a suitable aggregator for this port */
 	for (aggregator = __get_first_agg(port); aggregator;
 	     aggregator = __get_next_agg(aggregator)) {
 
-		// keep a free aggregator for later use(if needed)
+		/*/ keep a free aggregator for later use(if needed) */
 		if (!aggregator->lag_ports) {
 			if (!free_aggregator)
 				free_aggregator = aggregator;
 			continue;
 		}
-		// check if current aggregator suits us
-		if (((aggregator->actor_oper_aggregator_key == port->actor_oper_port_key) && // if all parameters match AND
-		     !MAC_ADDRESS_COMPARE(&(aggregator->partner_system), &(port->partner_oper.system)) &&
-		     (aggregator->partner_system_priority == port->partner_oper.system_priority) &&
-		     (aggregator->partner_oper_aggregator_key == port->partner_oper.key)
-		    ) &&
-		    ((MAC_ADDRESS_COMPARE(&(port->partner_oper.system), &(null_mac_addr)) && // partner answers
-		      !aggregator->is_individual)  // but is not individual OR
-		    )
-		   ) {
-			// attach to the founded aggregator
+		/* check if current aggregator suits us */
+		if (((aggregator->actor_oper_aggregator_key ==
+					      port->actor_oper_port_key)
+		      && !MAC_ADDRESS_COMPARE(&(aggregator->partner_system),
+						&(port->partner_oper.system))
+		      && (aggregator->partner_system_priority ==
+					port->partner_oper.system_priority)
+		      && (aggregator->partner_oper_aggregator_key ==
+						port->partner_oper.key))
+		    && ((MAC_ADDRESS_COMPARE(&(port->partner_oper.system),
+						&(null_mac_addr))
+		      && !aggregator->is_individual))) {
+			/* attach to the founded aggregator */
 			port->aggregator = aggregator;
 			port->actor_port_aggregator_identifier =
 				port->aggregator->aggregator_identifier;
@@ -1352,56 +1426,63 @@ static void ad_port_selection_logic(struct port *port)
 				 port->actor_port_number,
 				 port->aggregator->aggregator_identifier);
 
-			// mark this port as selected
+			/* mark this port as selected */
 			port->sm_vars |= AD_PORT_SELECTED;
 			found = 1;
 			break;
 		}
 	}
 
-	// the port couldn't find an aggregator - attach it to a new aggregator
+	/* the port couldn't find an aggregator,
+	   attach it to a new aggregator */
 	if (!found) {
 		if (free_aggregator) {
-			// assign port a new aggregator
+			/* assign port a new aggregator */
 			port->aggregator = free_aggregator;
 			port->actor_port_aggregator_identifier =
 				port->aggregator->aggregator_identifier;
 
-			// update the new aggregator's parameters
-			// if port was responsed from the end-user
+			/* update the new aggregator's parameters
+			   if port was responsed from the end-user */
 			if (port->actor_oper_port_key & AD_DUPLEX_KEY_BITS)
 				/* if port is full duplex */
 				port->aggregator->is_individual = false;
 			else
 				port->aggregator->is_individual = true;
 
-			port->aggregator->actor_admin_aggregator_key = port->actor_admin_port_key;
-			port->aggregator->actor_oper_aggregator_key = port->actor_oper_port_key;
+			port->aggregator->actor_admin_aggregator_key =
+						port->actor_admin_port_key;
+			port->aggregator->actor_oper_aggregator_key =
+						port->actor_oper_port_key;
 			port->aggregator->partner_system =
-				port->partner_oper.system;
+						port->partner_oper.system;
 			port->aggregator->partner_system_priority =
-				port->partner_oper.system_priority;
-			port->aggregator->partner_oper_aggregator_key = port->partner_oper.key;
+					port->partner_oper.system_priority;
+			port->aggregator->partner_oper_aggregator_key =
+						port->partner_oper.key;
 			port->aggregator->receive_state = 1;
 			port->aggregator->transmit_state = 1;
 			port->aggregator->lag_ports = port;
 			port->aggregator->num_of_ports++;
 
-			// mark this port as selected
+			/* mark this port as selected */
 			port->sm_vars |= AD_PORT_SELECTED;
 
 			pr_debug("Port %d joined LAG %d(new LAG)\n",
 				 port->actor_port_number,
 				 port->aggregator->aggregator_identifier);
 		} else {
-			pr_err("%s: Port %d (on %s) did not find a suitable aggregator\n",
-			       port->slave->dev->master->name,
+			pr_err("%s: Port %d (on %s) did not find a suitable "
+			       "aggregator\n", port->slave->dev->master->name,
 			       port->actor_port_number, port->slave->dev->name);
 		}
 	}
-	// if all aggregator's ports are READY_N == TRUE, set ready=TRUE in all aggregator's ports
-	// else set ready=FALSE in all aggregator's ports
-	__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
+	/* if all aggregator's ports are READY_N == TRUE,
+	 * set ready=TRUE in all aggregator's ports
+	 * else set ready=FALSE in all aggregator's ports
+	 */
+	__set_agg_ports_ready(port->aggregator,
+				__agg_ports_are_ready(port->aggregator));
 
 	aggregator = __get_first_agg(port);
 	ad_agg_selection_logic(aggregator);
@@ -1556,7 +1637,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 		active->is_active = 1;
 	}
 
-	// if there is new best aggregator, activate it
+	/* if there is new best aggregator, activate it */
 	if (best) {
 		pr_debug("best Agg=%d; P=%d; a k=%d; p k=%d; Ind=%d; Act=%d\n",
 			 best->aggregator_identifier, best->num_of_ports,
@@ -1577,9 +1658,10 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 				 agg->is_individual, agg->is_active);
 		}
 
-		// check if any partner replys
+		/* check if any partner replys */
 		if (best->is_individual) {
-			pr_warning("%s: Warning: No 802.3ad response from the link partner for any adapters in the bond\n",
+			pr_warning("%s: Warning: No 802.3ad response from the "
+				  "link partner for any adapters in the bond\n",
 				   best->slave ? best->slave->dev->master->name : "NULL");
 		}
 
@@ -1592,7 +1674,8 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 			 best->partner_oper_aggregator_key,
 			 best->is_individual, best->is_active);
 
-		// disable the ports that were related to the former active_aggregator
+		/* disable the ports that were related to
+		   the former active_aggregator */
 		if (active) {
 			for (port = active->lag_ports; port;
 			     port = port->next_port_in_aggregator) {
@@ -1701,8 +1784,10 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
 		port->ntt = false;
 		port->actor_admin_port_key = 1;
 		port->actor_oper_port_key  = 1;
-		port->actor_admin_port_state = AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
-		port->actor_oper_port_state  = AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
+		port->actor_admin_port_state =
+				AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
+		port->actor_oper_port_state  =
+				AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
 
 		if (lacp_fast)
 			port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
@@ -1711,7 +1796,7 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
 		memcpy(&port->partner_oper, &tmpl, sizeof(tmpl));
 
 		port->is_enabled = true;
-		// ****** private parameters ******
+		/* ****** private parameters ****** */
 		port->sm_vars = 0x3;
 		port->sm_rx_state = 0;
 		port->sm_rx_timer_counter = 0;
@@ -1753,7 +1838,9 @@ static void ad_enable_collecting_distributing(struct port *port)
  */
 static void ad_disable_collecting_distributing(struct port *port)
 {
-	if (port->aggregator && MAC_ADDRESS_COMPARE(&(port->aggregator->partner_system), &(null_mac_addr))) {
+	if (port->aggregator &&
+	    MAC_ADDRESS_COMPARE(&(port->aggregator->partner_system),
+	     &(null_mac_addr))) {
 		pr_debug("Disabling port %d(LAG %d)\n",
 			 port->actor_port_number,
 			 port->aggregator->aggregator_identifier);
@@ -1775,15 +1862,16 @@ static void ad_marker_info_send(struct port *port)
 	struct bond_marker marker;
 	u16 index;
 
-	// fill the marker PDU with the appropriate values
+	/* fill the marker PDU with the appropriate values */
 	marker.subtype = 0x02;
 	marker.version_number = 0x01;
 	marker.tlv_type = AD_MARKER_INFORMATION_SUBTYPE;
 	marker.marker_length = 0x16;
-	// convert requester_port to Big Endian
-	marker.requester_port = (((port->actor_port_number & 0xFF) << 8) |((u16)(port->actor_port_number & 0xFF00) >> 8));
+	/* convert requester_port to Big Endian */
+	marker.requester_port = (((port->actor_port_number & 0xFF) << 8) |
+				((u16)(port->actor_port_number & 0xFF00) >> 8));
 	marker.requester_system = port->actor_system;
-	// convert requester_port(u32) to Big Endian
+	/* convert requester_port(u32) to Big Endian */
 	marker.requester_transaction_id =
 		(((++port->transaction_id & 0xFF) << 24)
 		 | ((port->transaction_id & 0xFF00) << 8)
@@ -1795,7 +1883,7 @@ static void ad_marker_info_send(struct port *port)
 	for (index = 0; index < 90; index++)
 		marker.reserved_90[index] = 0;
 
-	// send the marker information
+	/* send the marker information */
 	if (ad_marker_send(port, &marker) >= 0) {
 		pr_debug("Sent Marker Information on port %d\n",
 			 port->actor_port_number);
@@ -1814,12 +1902,13 @@ static void ad_marker_info_received(struct bond_marker *marker_info,
 {
 	struct bond_marker marker;
 
-	// copy the received marker data to the response marker
-	//marker = *marker_info;
+	/* copy the received marker data to the response marker
+	 * marker = *marker_info;
+	 */
 	memcpy(&marker, marker_info, sizeof(struct bond_marker));
-	// change the marker subtype to marker response
+	/* change the marker subtype to marker response */
 	marker.tlv_type = AD_MARKER_RESPONSE_SUBTYPE;
-	// send the marker response
+	/* send the marker response */
 
 	if (ad_marker_send(port, &marker) >= 0) {
 		pr_debug("Sent Marker Response on port %d\n",
@@ -1839,16 +1928,13 @@ static void ad_marker_info_received(struct bond_marker *marker_info,
 static void ad_marker_response_received(struct bond_marker *marker,
 	struct port *port)
 {
-	marker = NULL; /* just to satisfy the compiler */
-	port = NULL;  /* just to satisfy the compiler */
-	// DO NOTHING, SINCE WE DECIDED NOT TO IMPLEMENT THIS FEATURE FOR NOW
+	marker = NULL;
+	port = NULL;
 }
 
-//////////////////////////////////////////////////////////////////////////////////////
-// ================= AD exported functions to the main bonding code ==================
-//////////////////////////////////////////////////////////////////////////////////////
+/* ============= AD exported functions to the main bonding code ============ */
 
-// Check aggregators status in team every T seconds
+/* Check aggregators status in team every T seconds */
 #define AD_AGGREGATOR_SELECTION_TIMER  8
 
 /*
@@ -1876,7 +1962,7 @@ static u16 aggregator_identifier;
  */
 void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution, int lacp_fast)
 {
-	// check that the bond is not initialized yet
+	/* check that the bond is not initialized yet */
 	if (MAC_ADDRESS_COMPARE(&(BOND_AD_INFO(bond).system.sys_mac_addr),
 				bond->dev->dev_addr)) {
 
@@ -1884,9 +1970,12 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution, int lacp_fas
 
 		BOND_AD_INFO(bond).lacp_fast = lacp_fast;
 		BOND_AD_INFO(bond).system.sys_priority = 0xFFFF;
-		BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
+		BOND_AD_INFO(bond).system.sys_mac_addr =
+				      *((struct mac_addr *)bond->dev->dev_addr);
 
-		// initialize how many times this module is called in one second(should be about every 100ms)
+		/* initialize how many times this module is
+		 * called in one second(should be about every 100ms)
+		 */
 		ad_ticks_per_sec = tick_resolution;
 
 		bond_3ad_initiate_agg_selection(bond,
@@ -1914,31 +2003,37 @@ int bond_3ad_bind_slave(struct slave *slave)
 		return -1;
 	}
 
-	//check that the slave has not been initialized yet.
+	/* check that the slave has not been initialized yet. */
 	if (SLAVE_AD_INFO(slave).port.slave != slave) {
 
-		// port initialization
+		/* port initialization */
 		port = &(SLAVE_AD_INFO(slave).port);
 
 		ad_initialize_port(port, BOND_AD_INFO(bond).lacp_fast);
 
 		port->slave = slave;
 		port->actor_port_number = SLAVE_AD_INFO(slave).id;
-		// key is determined according to the link speed, duplex and user key(which is yet not supported)
-		//              ------------------------------------------------------------
-		// Port key :   | User key                       |      Speed       |Duplex|
-		//              ------------------------------------------------------------
-		//              16                               6               1 0
-		port->actor_admin_port_key = 0;	// initialize this parameter
+		/* key is determined according to the link speed,
+		 * duplex and user key(which is yet not supported)
+		 * Port key:
+		 * ------------------------------------------------------------
+		 * | User key                       |      Speed       |Duplex|
+		 * ------------------------------------------------------------
+		 * 16                               6                  1      0
+		 */
+		port->actor_admin_port_key = 0;	/* initialize this parameter */
 		port->actor_admin_port_key |= __get_duplex(port);
 		port->actor_admin_port_key |= (__get_link_speed(port) << 1);
 		port->actor_oper_port_key = port->actor_admin_port_key;
-		// if the port is not full duplex, then the port should be not lacp Enabled
+		/* if the port is not full duplex,
+		 * then the port should be not lacp Enabled
+		 */
 		if (!(port->actor_oper_port_key & AD_DUPLEX_KEY_BITS))
 			port->sm_vars &= ~AD_PORT_LACP_ENABLED;
-		// actor system is the bond's system
+		/* actor system is the bond's system */
 		port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr;
-		// tx timer(to verify that no more than MAX_TX_IN_SECOND lacpdu's are sent in one second)
+		/* tx timer (to verify that no more
+		   than MAX_TX_IN_SECOND lacpdu's are sent in one second) */
 		port->sm_tx_timer_counter = ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
 		port->aggregator = NULL;
 		port->next_port_in_aggregator = NULL;
@@ -1947,12 +2042,13 @@ int bond_3ad_bind_slave(struct slave *slave)
 		__initialize_port_locks(port);
 
 
-		// aggregator initialization
+		/* aggregator initialization */
 		aggregator = &(SLAVE_AD_INFO(slave).aggregator);
 
 		ad_initialize_agg(aggregator);
 
-		aggregator->aggregator_mac_address = *((struct mac_addr *)bond->dev->dev_addr);
+		aggregator->aggregator_mac_address =
+				      *((struct mac_addr *)bond->dev->dev_addr);
 		aggregator->aggregator_identifier = (++aggregator_identifier);
 		aggregator->slave = slave;
 		aggregator->is_active = 0;
@@ -1976,16 +2072,17 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	struct aggregator *aggregator, *new_aggregator, *temp_aggregator;
 	int select_new_active_agg = 0;
 
-	// find the aggregator related to this slave
+	/* find the aggregator related to this slave */
 	aggregator = &(SLAVE_AD_INFO(slave).aggregator);
 
-	// find the port related to this slave
+	/* find the port related to this slave */
 	port = &(SLAVE_AD_INFO(slave).port);
 
-	// if slave is null, the whole port is not initialized
+	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
-		pr_warning("Warning: %s: Trying to unbind an uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+		pr_warning("Warning: %s: Trying to unbind an uninitialized "
+			   "port on %s\n", slave->dev->master->name,
+			   slave->dev->name);
 		return;
 	}
 
@@ -1997,99 +2094,135 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	__update_lacpdu_from_port(port);
 	ad_lacpdu_send(port);
 
-	// check if this aggregator is occupied
+	/* check if this aggregator is occupied */
 	if (aggregator->lag_ports) {
-		// check if there are other ports related to this aggregator except
-		// the port related to this slave(thats ensure us that there is a
-		// reason to search for new aggregator, and that we will find one
-		if ((aggregator->lag_ports != port) || (aggregator->lag_ports->next_port_in_aggregator)) {
-			// find new aggregator for the related port(s)
+		/* check if there are other ports related to this aggregator
+		 * except the port related to this slave (thats ensure us that
+		 * there is a reason to search for new aggregator, and that we
+		 * will find one
+		 */
+		if ((aggregator->lag_ports != port) ||
+		    (aggregator->lag_ports->next_port_in_aggregator)) {
+			/* find new aggregator for the related port(s) */
 			new_aggregator = __get_first_agg(port);
-			for (; new_aggregator; new_aggregator = __get_next_agg(new_aggregator)) {
-				// if the new aggregator is empty, or it is connected to our port only
+			for (; new_aggregator;
+			     new_aggregator = __get_next_agg(new_aggregator)) {
+				/* if the new aggregator is empty,
+				   or it is connected to our port only */
 				if (!new_aggregator->lag_ports
 				    || ((new_aggregator->lag_ports == port)
 					&& !new_aggregator->lag_ports->next_port_in_aggregator))
 					break;
 			}
-			// if new aggregator found, copy the aggregator's parameters
-			// and connect the related lag_ports to the new aggregator
-			if ((new_aggregator) && ((!new_aggregator->lag_ports) || ((new_aggregator->lag_ports == port) && !new_aggregator->lag_ports->next_port_in_aggregator))) {
-				pr_debug("Some port(s) related to LAG %d - replaceing with LAG %d\n",
+			/* if new aggregator found, copy the aggregator's
+			 * parameters and connect the related lag_ports to the
+			 * new aggregator
+			 */
+			if ((new_aggregator) &&
+			    ((!new_aggregator->lag_ports) ||
+			     ((new_aggregator->lag_ports == port) &&
+			      !new_aggregator->lag_ports->next_port_in_aggregator))) {
+				pr_debug("Some port(s) related to LAG %d - "
+					 "replaceing with LAG %d\n",
 					 aggregator->aggregator_identifier,
 					 new_aggregator->aggregator_identifier);
 
-				if ((new_aggregator->lag_ports == port) && new_aggregator->is_active) {
-					pr_info("%s: Removing an active aggregator\n",
+				if ((new_aggregator->lag_ports == port) &&
+				     new_aggregator->is_active) {
+					pr_info("%s: Removing an active "
+						"aggregator\n",
 						aggregator->slave->dev->master->name);
-					// select new active aggregator
+					/* select new active aggregator */
 					 select_new_active_agg = 1;
 				}
 
-				new_aggregator->is_individual = aggregator->is_individual;
-				new_aggregator->actor_admin_aggregator_key = aggregator->actor_admin_aggregator_key;
-				new_aggregator->actor_oper_aggregator_key = aggregator->actor_oper_aggregator_key;
-				new_aggregator->partner_system = aggregator->partner_system;
-				new_aggregator->partner_system_priority = aggregator->partner_system_priority;
-				new_aggregator->partner_oper_aggregator_key = aggregator->partner_oper_aggregator_key;
-				new_aggregator->receive_state = aggregator->receive_state;
-				new_aggregator->transmit_state = aggregator->transmit_state;
-				new_aggregator->lag_ports = aggregator->lag_ports;
-				new_aggregator->is_active = aggregator->is_active;
-				new_aggregator->num_of_ports = aggregator->num_of_ports;
-
-				// update the information that is written on the ports about the aggregator
-				for (temp_port = aggregator->lag_ports; temp_port;
-				     temp_port = temp_port->next_port_in_aggregator) {
+				new_aggregator->is_individual =
+						      aggregator->is_individual;
+				new_aggregator->actor_admin_aggregator_key =
+					 aggregator->actor_admin_aggregator_key;
+				new_aggregator->actor_oper_aggregator_key =
+					  aggregator->actor_oper_aggregator_key;
+				new_aggregator->partner_system =
+						     aggregator->partner_system;
+				new_aggregator->partner_system_priority =
+					    aggregator->partner_system_priority;
+				new_aggregator->partner_oper_aggregator_key =
+					aggregator->partner_oper_aggregator_key;
+				new_aggregator->receive_state =
+						      aggregator->receive_state;
+				new_aggregator->transmit_state =
+						     aggregator->transmit_state;
+				new_aggregator->lag_ports =
+							  aggregator->lag_ports;
+				new_aggregator->is_active =
+							  aggregator->is_active;
+				new_aggregator->num_of_ports =
+						       aggregator->num_of_ports;
+
+				/* update the information that is written on
+				 * the ports about the aggregator
+				 */
+				for (temp_port = aggregator->lag_ports;
+				     temp_port; temp_port = temp_port->next_port_in_aggregator) {
 					temp_port->aggregator = new_aggregator;
 					temp_port->actor_port_aggregator_identifier = new_aggregator->aggregator_identifier;
 				}
 
-				// clear the aggregator
+				/* clear the aggregator */
 				ad_clear_agg(aggregator);
 
 				if (select_new_active_agg)
 					ad_agg_selection_logic(__get_first_agg(port));
 			} else {
-				pr_warning("%s: Warning: unbinding aggregator, and could not find a new aggregator for its ports\n",
+				pr_warning("%s: Warning: unbinding aggregator, "
+					   "and could not find a new aggregator"
+					   " for its ports\n",
 					   slave->dev->master->name);
 			}
-		} else { // in case that the only port related to this aggregator is the one we want to remove
+		} else {
+			 /* in case that the only port related to this
+			  * aggregator is the one we want to remove
+			  */
 			select_new_active_agg = aggregator->is_active;
-			// clear the aggregator
+			/* clear the aggregator */
 			ad_clear_agg(aggregator);
 			if (select_new_active_agg) {
 				pr_info("%s: Removing an active aggregator\n",
 					slave->dev->master->name);
-				// select new active aggregator
+				/* select new active aggregator */
 				ad_agg_selection_logic(__get_first_agg(port));
 			}
 		}
 	}
 
 	pr_debug("Unbinding port %d\n", port->actor_port_number);
-	// find the aggregator that this port is connected to
+	/* find the aggregator that this port is connected to */
 	temp_aggregator = __get_first_agg(port);
-	for (; temp_aggregator; temp_aggregator = __get_next_agg(temp_aggregator)) {
+	for (; temp_aggregator;
+	     temp_aggregator = __get_next_agg(temp_aggregator)) {
 		prev_port = NULL;
-		// search the port in the aggregator's related ports
+		/* search the port in the aggregator's related ports */
 		for (temp_port = temp_aggregator->lag_ports; temp_port;
 		     prev_port = temp_port,
 			     temp_port = temp_port->next_port_in_aggregator) {
-			if (temp_port == port) { // the aggregator found - detach the port from this aggregator
+			if (temp_port == port) {
+				/* the aggregator found
+				   detach the port from this aggregator */
 				if (prev_port)
-					prev_port->next_port_in_aggregator = temp_port->next_port_in_aggregator;
+					prev_port->next_port_in_aggregator =
+					     temp_port->next_port_in_aggregator;
 				else
-					temp_aggregator->lag_ports = temp_port->next_port_in_aggregator;
+					temp_aggregator->lag_ports =
+					     temp_port->next_port_in_aggregator;
 				temp_aggregator->num_of_ports--;
 				if (temp_aggregator->num_of_ports == 0) {
 					select_new_active_agg = temp_aggregator->is_active;
-					// clear the aggregator
+					/* clear the aggregator */
 					ad_clear_agg(temp_aggregator);
 					if (select_new_active_agg) {
 						pr_info("%s: Removing an active aggregator\n",
 							slave->dev->master->name);
-						// select new active aggregator
+						/* select new active aggreg */
 						ad_agg_selection_logic(__get_first_agg(port));
 					}
 				}
@@ -2125,17 +2258,18 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 	if (bond->kill_timers)
 		goto out;
 
-	//check if there are any slaves
+	/* check if there are any slaves */
 	if (bond->slave_cnt == 0)
 		goto re_arm;
 
-	// check if agg_select_timer timer after initialize is timed out
-	if (BOND_AD_INFO(bond).agg_select_timer && !(--BOND_AD_INFO(bond).agg_select_timer)) {
-		// select the active aggregator for the bond
+	/* check if agg_select_timer timer after initialize is timed out */
+	if (BOND_AD_INFO(bond).agg_select_timer &&
+	    !(--BOND_AD_INFO(bond).agg_select_timer)) {
+		/* select the active aggregator for the bond */
 		if ((port = __get_first_port(bond))) {
 			if (!port->slave) {
-				pr_warning("%s: Warning: bond's first port is uninitialized\n",
-					   bond->dev->name);
+				pr_warning("%s: Warning: bond's first port is "
+					   "uninitialized\n", bond->dev->name);
 				goto re_arm;
 			}
 
@@ -2145,8 +2279,9 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 		bond_3ad_set_carrier(bond);
 	}
 
-	// for each port run the state machines
-	for (port = __get_first_port(bond); port; port = __get_next_port(port)) {
+	/* for each port run the state machines */
+	for (port = __get_first_port(bond); port;
+	     port = __get_next_port(port)) {
 		if (!port->slave) {
 			pr_warning("%s: Warning: Found an uninitialized port\n",
 				   bond->dev->name);
@@ -2165,7 +2300,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 		ad_mux_machine(port);
 		ad_tx_machine(port);
 
-		// turn off the BEGIN bit, since we already handled it
+		/* turn off the BEGIN bit, since we already handled it */
 		if (port->sm_vars & AD_PORT_BEGIN)
 			port->sm_vars &= ~AD_PORT_BEGIN;
 
@@ -2197,8 +2332,9 @@ static void bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, u
 		port = &(SLAVE_AD_INFO(slave).port);
 
 		if (!port->slave) {
-			pr_warning("%s: Warning: port of slave %s is uninitialized\n",
-				   slave->dev->name, slave->dev->master->name);
+			pr_warning("%s: Warning: port of slave %s is "
+				   "uninitialized\n", slave->dev->name,
+						      slave->dev->master->name);
 			return;
 		}
 
@@ -2213,24 +2349,26 @@ static void bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, u
 			break;
 
 		case AD_TYPE_MARKER:
-			// No need to convert fields to Little Endian since we don't use the marker's fields.
+			/* No need to convert fields to Little Endian
+			 *  since we don't use the marker's fields.
+			 */
 
 			switch (((struct bond_marker *)lacpdu)->tlv_type) {
 			case AD_MARKER_INFORMATION_SUBTYPE:
-				pr_debug("Received Marker Information on port %d\n",
-					 port->actor_port_number);
+				pr_debug("Received Marker Information on "
+					 "port %d\n", port->actor_port_number);
 				ad_marker_info_received((struct bond_marker *)lacpdu, port);
 				break;
 
 			case AD_MARKER_RESPONSE_SUBTYPE:
-				pr_debug("Received Marker Response on port %d\n",
-					 port->actor_port_number);
+				pr_debug("Received Marker Response on "
+					 "port %d\n", port->actor_port_number);
 				ad_marker_response_received((struct bond_marker *)lacpdu, port);
 				break;
 
 			default:
-				pr_debug("Received an unknown Marker subtype on slot %d\n",
-					 port->actor_port_number);
+				pr_debug("Received an unknown Marker subtype "
+					 "on slot %d\n", port->actor_port_number);
 			}
 		}
 	}
@@ -2248,10 +2386,11 @@ void bond_3ad_adapter_speed_changed(struct slave *slave)
 
 	port = &(SLAVE_AD_INFO(slave).port);
 
-	// if slave is null, the whole port is not initialized
+	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
-		pr_warning("Warning: %s: speed changed for uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+		pr_warning("Warning: %s: speed changed for uninitialized "
+			   "port on %s\n", slave->dev->master->name,
+							      slave->dev->name);
 		return;
 	}
 
@@ -2259,8 +2398,8 @@ void bond_3ad_adapter_speed_changed(struct slave *slave)
 	port->actor_oper_port_key = port->actor_admin_port_key |=
 		(__get_link_speed(port) << 1);
 	pr_debug("Port %d changed speed\n", port->actor_port_number);
-	// there is no need to reselect a new aggregator, just signal the
-	// state machines to reinitialize
+	/* there is no need to reselect a new aggregator, just signal the
+	   state machines to reinitialize */
 	port->sm_vars |= AD_PORT_BEGIN;
 }
 
@@ -2276,10 +2415,11 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
 
 	port = &(SLAVE_AD_INFO(slave).port);
 
-	// if slave is null, the whole port is not initialized
+	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
-		pr_warning("%s: Warning: duplex changed for uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+		pr_warning("%s: Warning: duplex changed for uninitialized "
+			   "port on %s\n", slave->dev->master->name,
+							      slave->dev->name);
 		return;
 	}
 
@@ -2287,8 +2427,8 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
 	port->actor_oper_port_key = port->actor_admin_port_key |=
 		__get_duplex(port);
 	pr_debug("Port %d changed duplex\n", port->actor_port_number);
-	// there is no need to reselect a new aggregator, just signal the
-	// state machines to reinitialize
+	/* there is no need to reselect a new aggregator, just signal the
+	   state machines to reinitialize */
 	port->sm_vars |= AD_PORT_BEGIN;
 }
 
@@ -2305,15 +2445,19 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 
 	port = &(SLAVE_AD_INFO(slave).port);
 
-	// if slave is null, the whole port is not initialized
+	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
-		pr_warning("Warning: %s: link status changed for uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+		pr_warning("Warning: %s: link status changed for uninitialized "
+			   "port on %s\n", slave->dev->master->name,
+							      slave->dev->name);
 		return;
 	}
 
-	// on link down we are zeroing duplex and speed since some of the adaptors(ce1000.lan) report full duplex/speed instead of N/A(duplex) / 0(speed)
-	// on link up we are forcing recheck on the duplex and speed since some of he adaptors(ce1000.lan) report
+	/* on link down we are zeroing duplex and speed since some of the
+	 * adaptors(ce1000.lan) report full duplex/speed instead of N/A
+	 * (duplex) / 0(speed) on link up we are forcing recheck on
+	 * the duplex and speed since some of he adaptors (ce1000.lan) report
+	 */
 	if (link == BOND_LINK_UP) {
 		port->is_enabled = true;
 		port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS;
@@ -2329,9 +2473,14 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 		port->actor_oper_port_key = (port->actor_admin_port_key &=
 					     ~AD_SPEED_KEY_BITS);
 	}
-	//BOND_PRINT_DBG(("Port %d changed link status to %s", port->actor_port_number, ((link == BOND_LINK_UP)?"UP":"DOWN")));
-	// there is no need to reselect a new aggregator, just signal the
-	// state machines to reinitialize
+	/*
+	 BOND_PRINT_DBG(("Port %d changed link status to %s",
+			  port->actor_port_number,
+			  ((link == BOND_LINK_UP)?"UP":"DOWN")));
+	 */
+
+	/* there is no need to reselect a new aggregator, just signal the
+	   state machines to reinitialize */
 	port->sm_vars |= AD_PORT_BEGIN;
 }
 
@@ -2387,7 +2536,8 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
 		ad_info->ports = aggregator->num_of_ports;
 		ad_info->actor_key = aggregator->actor_oper_aggregator_key;
 		ad_info->partner_key = aggregator->partner_oper_aggregator_key;
-		memcpy(ad_info->partner_system, aggregator->partner_system.mac_addr_value, ETH_ALEN);
+		memcpy(ad_info->partner_system,
+			 aggregator->partner_system.mac_addr_value, ETH_ALEN);
 		return 0;
 	}
 
@@ -2441,8 +2591,8 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	if (slave_agg_no >= 0) {
-		pr_err("%s: Error: Couldn't find a slave to tx on for aggregator ID %d\n",
-		       dev->name, agg_id);
+		pr_err("%s: Error: Couldn't find a slave to tx on for "
+			"aggregator ID %d\n", dev->name, agg_id);
 		goto out;
 	}
 
diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
index b28baff..55c8f42 100644
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@ -28,7 +28,7 @@
 #include <linux/netdevice.h>
 #include <linux/if_ether.h>
 
-// General definitions
+/* General definitions */
 #define PKT_TYPE_LACPDU         cpu_to_be16(ETH_P_SLOW)
 #define AD_TIMER_INTERVAL       100 /*msec*/
 
@@ -47,54 +47,54 @@ enum {
 	BOND_AD_COUNT = 2,
 };
 
-// rx machine states(43.4.11 in the 802.3ad standard)
+/* rx machine states(43.4.11 in the 802.3ad standard) */
 typedef enum {
 	AD_RX_DUMMY,
-	AD_RX_INITIALIZE,     // rx Machine
-	AD_RX_PORT_DISABLED,  // rx Machine
-	AD_RX_LACP_DISABLED,  // rx Machine
-	AD_RX_EXPIRED,	      // rx Machine
-	AD_RX_DEFAULTED,      // rx Machine
-	AD_RX_CURRENT	      // rx Machine
+	AD_RX_INITIALIZE,
+	AD_RX_PORT_DISABLED,
+	AD_RX_LACP_DISABLED,
+	AD_RX_EXPIRED,
+	AD_RX_DEFAULTED,
+	AD_RX_CURRENT
 } rx_states_t;
 
-// periodic machine states(43.4.12 in the 802.3ad standard)
+/* periodic machine states(43.4.12 in the 802.3ad standard) */
 typedef enum {
 	AD_PERIODIC_DUMMY,
-	AD_NO_PERIODIC,	       // periodic machine
-	AD_FAST_PERIODIC,      // periodic machine
-	AD_SLOW_PERIODIC,      // periodic machine
-	AD_PERIODIC_TX	   // periodic machine
+	AD_NO_PERIODIC,
+	AD_FAST_PERIODIC,
+	AD_SLOW_PERIODIC,
+	AD_PERIODIC_TX
 } periodic_states_t;
 
-// mux machine states(43.4.13 in the 802.3ad standard)
+/* mux machine states(43.4.13 in the 802.3ad standard) */
 typedef enum {
 	AD_MUX_DUMMY,
-	AD_MUX_DETACHED,       // mux machine
-	AD_MUX_WAITING,	       // mux machine
-	AD_MUX_ATTACHED,       // mux machine
-	AD_MUX_COLLECTING_DISTRIBUTING // mux machine
+	AD_MUX_DETACHED,
+	AD_MUX_WAITING,
+	AD_MUX_ATTACHED,
+	AD_MUX_COLLECTING_DISTRIBUTING
 } mux_states_t;
 
-// tx machine states(43.4.15 in the 802.3ad standard)
+/* tx machine states(43.4.15 in the 802.3ad standard) */
 typedef enum {
 	AD_TX_DUMMY,
-	AD_TRANSMIT	   // tx Machine
+	AD_TRANSMIT
 } tx_states_t;
 
-// rx indication types
+/* rx indication types */
 typedef enum {
-	AD_TYPE_LACPDU = 1,    // type lacpdu
-	AD_TYPE_MARKER	   // type marker
+	AD_TYPE_LACPDU = 1,
+	AD_TYPE_MARKER
 } pdu_type_t;
 
-// rx marker indication types
+/* rx marker indication types */
 typedef enum {
-	AD_MARKER_INFORMATION_SUBTYPE = 1, // marker imformation subtype
-	AD_MARKER_RESPONSE_SUBTYPE     // marker response subtype
+	AD_MARKER_INFORMATION_SUBTYPE = 1,
+	AD_MARKER_RESPONSE_SUBTYPE
 } bond_marker_subtype_t;
 
-// timers types(43.4.9 in the 802.3ad standard)
+/* timers types(43.4.9 in the 802.3ad standard) */
 typedef enum {
 	AD_CURRENT_WHILE_TIMER,
 	AD_ACTOR_CHURN_TIMER,
@@ -105,35 +105,37 @@ typedef enum {
 
 #pragma pack(1)
 
-// Link Aggregation Control Protocol(LACP) data unit structure(43.4.2.2 in the 802.3ad standard)
+/* Link Aggregation Control Protocol(LACP) data unit
+ * structure(43.4.2.2 in the 802.3ad standard)
+ */
 typedef struct lacpdu {
-	u8 subtype;		     // = LACP(= 0x01)
+	u8 subtype;		          /* = LACP(= 0x01) */
 	u8 version_number;
-	u8 tlv_type_actor_info;	      // = actor information(type/length/value)
-	u8 actor_information_length; // = 20
+	u8 tlv_type_actor_info;	          /* = actor info(type/length/value)*/
+	u8 actor_information_length;      /* = 20 */
 	__be16 actor_system_priority;
 	struct mac_addr actor_system;
 	__be16 actor_key;
 	__be16 actor_port_priority;
 	__be16 actor_port;
 	u8 actor_state;
-	u8 reserved_3_1[3];	     // = 0
-	u8 tlv_type_partner_info;     // = partner information
-	u8 partner_information_length;	 // = 20
+	u8 reserved_3_1[3];	          /* = 0 */
+	u8 tlv_type_partner_info;         /* = partner information */
+	u8 partner_information_length;	  /* = 20 */
 	__be16 partner_system_priority;
 	struct mac_addr partner_system;
 	__be16 partner_key;
 	__be16 partner_port_priority;
 	__be16 partner_port;
 	u8 partner_state;
-	u8 reserved_3_2[3];	     // = 0
-	u8 tlv_type_collector_info;	  // = collector information
-	u8 collector_information_length; // = 16
+	u8 reserved_3_2[3];	          /* = 0 */
+	u8 tlv_type_collector_info;	  /* = collector information */
+	u8 collector_information_length;  /* = 16 */
 	__be16 collector_max_delay;
 	u8 reserved_12[12];
-	u8 tlv_type_terminator;	     // = terminator
-	u8 terminator_length;	     // = 0
-	u8 reserved_50[50];	     // = 0
+	u8 tlv_type_terminator;	          /* = terminator */
+	u8 terminator_length;	          /* = 0 */
+	u8 reserved_50[50];	          /* = 0 */
 } lacpdu_t;
 
 typedef struct lacpdu_header {
@@ -141,20 +143,20 @@ typedef struct lacpdu_header {
 	struct lacpdu lacpdu;
 } lacpdu_header_t;
 
-// Marker Protocol Data Unit(PDU) structure(43.5.3.2 in the 802.3ad standard)
+/* Marker Protocol Data Unit(PDU) structure(43.5.3.2 in the 802.3ad standard) */
 typedef struct bond_marker {
-	u8 subtype;		 //  = 0x02  (marker PDU)
-	u8 version_number;	 //  = 0x01
-	u8 tlv_type;		 //  = 0x01  (marker information)
-	//  = 0x02  (marker response information)
-	u8 marker_length;	 //  = 0x16
-	u16 requester_port;	 //   The number assigned to the port by the requester
-	struct mac_addr requester_system;      //   The requester's system id
-	u32 requester_transaction_id;	//   The transaction id allocated by the requester,
-	u16 pad;		 //  = 0
-	u8 tlv_type_terminator;	     //  = 0x00
-	u8 terminator_length;	     //  = 0x00
-	u8 reserved_90[90];	     //  = 0
+	u8 subtype;			  /* = 0x02 (marker PDU) */
+	u8 version_number;		  /* = 0x01 */
+	u8 tlv_type;			  /* = 0x01 (marker information)
+					   * = 0x02  (marker response info */
+	u8 marker_length;		  /* = 0x16 */
+	u16 requester_port;
+	struct mac_addr requester_system; /* The requester's system id */
+	u32 requester_transaction_id;
+	u16 pad;			  /* = 0 */
+	u8 tlv_type_terminator;		  /* = 0x00 */
+	u8 terminator_length;		  /* = 0x00 */
+	u8 reserved_90[90];		  /* = 0 */
 } bond_marker_t;
 
 typedef struct bond_marker_header {
@@ -173,7 +175,7 @@ struct port;
 #pragma pack(8)
 #endif
 
-// aggregator structure(43.4.5 in the 802.3ad standard)
+/* aggregator structure(43.4.5 in the 802.3ad standard) */
 typedef struct aggregator {
 	struct mac_addr aggregator_mac_address;
 	u16 aggregator_identifier;
@@ -183,12 +185,13 @@ typedef struct aggregator {
 	struct mac_addr partner_system;
 	u16 partner_system_priority;
 	u16 partner_oper_aggregator_key;
-	u16 receive_state;		// BOOLEAN
-	u16 transmit_state;		// BOOLEAN
+	u16 receive_state;		/* BOOLEAN */
+	u16 transmit_state;		/* BOOLEAN */
 	struct port *lag_ports;
-	// ****** PRIVATE PARAMETERS ******
-	struct slave *slave;	    // pointer to the bond slave that this aggregator belongs to
-	u16 is_active;	    // BOOLEAN. Indicates if this aggregator is active
+	/* ****** PRIVATE PARAMETERS ****** */
+	struct slave *slave; /* poiter to the bond slave
+				that this aggregator belongs to */
+	u16 is_active;	     /* BOOLEAN. Indicates if the aggregator is active*/
 	u16 num_of_ports;
 } aggregator_t;
 
@@ -201,12 +204,18 @@ struct port_params {
 	u16 port_state;
 };
 
-// port structure(43.4.6 in the 802.3ad standard)
+/* port structure(43.4.6 in the 802.3ad standard) */
 typedef struct port {
 	u16 actor_port_number;
 	u16 actor_port_priority;
-	struct mac_addr actor_system;	       // This parameter is added here although it is not specified in the standard, just for simplification
-	u16 actor_system_priority;	 // This parameter is added here although it is not specified in the standard, just for simplification
+
+	/* in a attempt to simplify operations the
+	 * following two elements were included here
+	 * despite they are not specified in the standard
+	 */
+	struct mac_addr actor_system;
+	u16 actor_system_priority;
+
 	u16 actor_port_aggregator_identifier;
 	bool ntt;
 	u16 actor_admin_port_key;
@@ -219,21 +228,21 @@ typedef struct port {
 
 	bool is_enabled;
 
-	// ****** PRIVATE PARAMETERS ******
-	u16 sm_vars;	      // all state machines variables for this port
-	rx_states_t sm_rx_state;	// state machine rx state
-	u16 sm_rx_timer_counter;    // state machine rx timer counter
-	periodic_states_t sm_periodic_state;// state machine periodic state
-	u16 sm_periodic_timer_counter;	// state machine periodic timer counter
-	mux_states_t sm_mux_state;	// state machine mux state
-	u16 sm_mux_timer_counter;   // state machine mux timer counter
-	tx_states_t sm_tx_state;	// state machine tx state
-	u16 sm_tx_timer_counter;    // state machine tx timer counter(allways on - enter to transmit state 3 time per second)
-	struct slave *slave;	    // pointer to the bond slave that this port belongs to
-	struct aggregator *aggregator;	   // pointer to an aggregator that this port related to
-	struct port *next_port_in_aggregator; // Next port on the linked list of the parent aggregator
-	u32 transaction_id;	    // continuous number for identification of Marker PDU's;
-	struct lacpdu lacpdu;	       // the lacpdu that will be sent for this port
+	/* ****** PRIVATE PARAMETERS ****** */
+	u16 sm_vars;
+	rx_states_t sm_rx_state;
+	u16 sm_rx_timer_counter;
+	periodic_states_t sm_periodic_state;
+	u16 sm_periodic_timer_counter;
+	mux_states_t sm_mux_state;
+	u16 sm_mux_timer_counter;
+	tx_states_t sm_tx_state;
+	u16 sm_tx_timer_counter;
+	struct slave *slave;
+	struct aggregator *aggregator;
+	struct port *next_port_in_aggregator;
+	u32 transaction_id;
+	struct lacpdu lacpdu;
 } port_t;
 
 // system structure
@@ -246,31 +255,30 @@ struct ad_system {
 #endif
 
-// ================= AD Exported structures to the main bonding code ==================
+/* =========== AD Exported structures to the main bonding code ============ */
 #define BOND_AD_INFO(bond)   ((bond)->ad_info)
 #define SLAVE_AD_INFO(slave) ((slave)->ad_info)
 
 struct ad_bond_info {
 	struct ad_system system;	    /* 802.3ad system structure */
-	u32 agg_select_timer;	    // Timer to select aggregator after all adapter's hand shakes
-	u32 agg_select_mode;	    // Mode of selection of active aggregator(bandwidth/count)
-	int lacp_fast;		/* whether fast periodic tx should be
-				 * requested
-				 */
+	u32 agg_select_timer;	            /* aggregator's selected timer */
+	u32 agg_select_mode;	            /* aggregator's selected mode */
+	int lacp_fast;
 	struct timer_list ad_timer;
 	struct packet_type ad_pkt_type;
 };
 
 struct ad_slave_info {
-	struct aggregator aggregator;	    // 802.3ad aggregator structure
-	struct port port;		    // 802.3ad port structure
-	spinlock_t state_machine_lock; /* mutex state machines vs.
-					  incoming LACPDU */
+	struct aggregator aggregator;	    /* 802.3ad aggregator structure */
+	struct port port;		    /* 802.3ad port structure */
+	spinlock_t state_machine_lock;	    /* mutex state machines vs.
+					     * incoming LACPDU */
 	u16 id;
 };
 
-// ================= AD Exported functions to the main bonding code ==================
-void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution, int lacp_fast);
+/* ========= AD Exported functions to the main bonding code ========== */
+void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution,
+								 int lacp_fast);
 int  bond_3ad_bind_slave(struct slave *slave);
 void bond_3ad_unbind_slave(struct slave *slave);
 void bond_3ad_state_machine_handler(struct work_struct *);
@@ -280,7 +288,8 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave);
 void bond_3ad_handle_link_change(struct slave *slave, char link);
 int  bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info);
 int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev);
-int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type* ptype, struct net_device *orig_dev);
+int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev,
+			struct packet_type* ptype, struct net_device *orig_dev);
 int bond_3ad_set_carrier(struct bonding *bond);
 #endif //__BOND_3AD_H__
 
-- 
1.7.4.4

^ permalink raw reply related

* Re: [PATCH 2/2] libertas: Use netdev_<level> or dev_<level> where possible
From: Dan Williams @ 2011-05-04 22:15 UTC (permalink / raw)
  To: Joe Perches
  Cc: John W. Linville, libertas-dev, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <b4b100451cb09b60065f2cf87b893b8d42759aaf.1304379925.git.joe@perches.com>

On Mon, 2011-05-02 at 16:49 -0700, Joe Perches wrote:
> Using the more descriptive logging styles gives a bit
> more information about the device being operated on.
> 
> Makes the object trivially smaller too.
> 
> $ size drivers/net/wireless/libertas/built-in.o.*
>  187730	   2973	  38488	 229191	  37f47	drivers/net/wireless/libertas/built-in.o.new
>  188195	   2973	  38488	 229656	  38118	drivers/net/wireless/libertas/built-in.o.old
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Acked-by: Dan Williams <dcbw@redhat.com>

> ---
>  drivers/net/wireless/libertas/cfg.c     |    9 +++--
>  drivers/net/wireless/libertas/cmd.c     |   42 +++++++++++++----------
>  drivers/net/wireless/libertas/cmdresp.c |   32 +++++++++---------
>  drivers/net/wireless/libertas/debugfs.c |    7 ++--
>  drivers/net/wireless/libertas/if_cs.c   |   15 +++++---
>  drivers/net/wireless/libertas/if_sdio.c |   19 +++++-----
>  drivers/net/wireless/libertas/if_spi.c  |   57 ++++++++++++++++++++-----------
>  drivers/net/wireless/libertas/if_usb.c  |   11 ++++--
>  drivers/net/wireless/libertas/main.c    |   27 +++++++++------
>  drivers/net/wireless/libertas/mesh.c    |    4 +-
>  drivers/net/wireless/libertas/rx.c      |    2 +-
>  11 files changed, 131 insertions(+), 94 deletions(-)
> 
> diff --git a/drivers/net/wireless/libertas/cfg.c b/drivers/net/wireless/libertas/cfg.c
> index d6e0656..47b7f17 100644
> --- a/drivers/net/wireless/libertas/cfg.c
> +++ b/drivers/net/wireless/libertas/cfg.c
> @@ -1316,7 +1316,8 @@ static int lbs_cfg_connect(struct wiphy *wiphy, struct net_device *dev,
>  		sme->ssid, sme->ssid_len,
>  		WLAN_CAPABILITY_ESS, WLAN_CAPABILITY_ESS);
>  	if (!bss) {
> -		pr_err("assoc: bss %pM not in scan results\n", sme->bssid);
> +		wiphy_err(wiphy, "assoc: bss %pM not in scan results\n",
> +			  sme->bssid);
>  		ret = -ENOENT;
>  		goto done;
>  	}
> @@ -1373,8 +1374,8 @@ static int lbs_cfg_connect(struct wiphy *wiphy, struct net_device *dev,
>  		lbs_enable_rsn(priv, sme->crypto.cipher_group != 0);
>  		break;
>  	default:
> -		pr_err("unsupported cipher group 0x%x\n",
> -		       sme->crypto.cipher_group);
> +		wiphy_err(wiphy, "unsupported cipher group 0x%x\n",
> +			  sme->crypto.cipher_group);
>  		ret = -ENOTSUPP;
>  		goto done;
>  	}
> @@ -1492,7 +1493,7 @@ static int lbs_cfg_add_key(struct wiphy *wiphy, struct net_device *netdev,
>  				     params->key, params->key_len);
>  		break;
>  	default:
> -		pr_err("unhandled cipher 0x%x\n", params->cipher);
> +		wiphy_err(wiphy, "unhandled cipher 0x%x\n", params->cipher);
>  		ret = -ENOTSUPP;
>  		break;
>  	}
> diff --git a/drivers/net/wireless/libertas/cmd.c b/drivers/net/wireless/libertas/cmd.c
> index 10ca485..f11c656 100644
> --- a/drivers/net/wireless/libertas/cmd.c
> +++ b/drivers/net/wireless/libertas/cmd.c
> @@ -3,8 +3,6 @@
>    * It prepares command and sends it to firmware when it is ready.
>    */
>  
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
>  #include <linux/kfifo.h>
>  #include <linux/sched.h>
>  #include <linux/slab.h>
> @@ -110,7 +108,7 @@ int lbs_update_hw_spec(struct lbs_private *priv)
>  	 * CF card    firmware 5.0.16p0:   cap 0x00000303
>  	 * USB dongle firmware 5.110.17p2: cap 0x00000303
>  	 */
> -	pr_info("%pM, fw %u.%u.%up%u, cap 0x%08x\n",
> +	netdev_info(priv->dev, "%pM, fw %u.%u.%up%u, cap 0x%08x\n",
>  		cmd.permanentaddr,
>  		priv->fwrelease >> 24 & 0xff,
>  		priv->fwrelease >> 16 & 0xff,
> @@ -141,7 +139,8 @@ int lbs_update_hw_spec(struct lbs_private *priv)
>  	/* if it's unidentified region code, use the default (USA) */
>  	if (i >= MRVDRV_MAX_REGION_CODE) {
>  		priv->regioncode = 0x10;
> -		pr_info("unidentified region code; using the default (USA)\n");
> +		netdev_info(priv->dev,
> +			    "unidentified region code; using the default (USA)\n");
>  	}
>  
>  	if (priv->current_addr[0] == 0xff)
> @@ -211,7 +210,7 @@ int lbs_host_sleep_cfg(struct lbs_private *priv, uint32_t criteria,
>  					(uint8_t *)&cmd_config.wol_conf,
>  					sizeof(struct wol_config));
>  	} else {
> -		pr_info("HOST_SLEEP_CFG failed %d\n", ret);
> +		netdev_info(priv->dev, "HOST_SLEEP_CFG failed %d\n", ret);
>  	}
>  
>  	return ret;
> @@ -314,7 +313,7 @@ static int lbs_wait_for_ds_awake(struct lbs_private *priv)
>  	if (priv->is_deep_sleep) {
>  		if (!wait_event_interruptible_timeout(priv->ds_awake_q,
>  					!priv->is_deep_sleep, (10 * HZ))) {
> -			pr_err("ds_awake_q: timer expired\n");
> +			netdev_err(priv->dev, "ds_awake_q: timer expired\n");
>  			ret = -1;
>  		}
>  	}
> @@ -339,7 +338,7 @@ int lbs_set_deep_sleep(struct lbs_private *priv, int deep_sleep)
>  				netif_carrier_off(priv->dev);
>  			}
>  		} else {
> -			pr_err("deep sleep: already enabled\n");
> +			netdev_err(priv->dev, "deep sleep: already enabled\n");
>  		}
>  	} else {
>  		if (priv->is_deep_sleep) {
> @@ -349,7 +348,8 @@ int lbs_set_deep_sleep(struct lbs_private *priv, int deep_sleep)
>  			if (!ret) {
>  				ret = lbs_wait_for_ds_awake(priv);
>  				if (ret)
> -					pr_err("deep sleep: wakeup failed\n");
> +					netdev_err(priv->dev,
> +						   "deep sleep: wakeup failed\n");
>  			}
>  		}
>  	}
> @@ -383,8 +383,9 @@ int lbs_set_host_sleep(struct lbs_private *priv, int host_sleep)
>  			ret = lbs_host_sleep_cfg(priv, priv->wol_criteria,
>  					(struct wol_config *)NULL);
>  			if (ret) {
> -				pr_info("Host sleep configuration failed: %d\n",
> -					ret);
> +				netdev_info(priv->dev,
> +					    "Host sleep configuration failed: %d\n",
> +					    ret);
>  				return ret;
>  			}
>  			if (priv->psstate == PS_STATE_FULL_POWER) {
> @@ -394,19 +395,21 @@ int lbs_set_host_sleep(struct lbs_private *priv, int host_sleep)
>  						sizeof(cmd),
>  						lbs_ret_host_sleep_activate, 0);
>  				if (ret)
> -					pr_info("HOST_SLEEP_ACTIVATE failed: %d\n",
> -						ret);
> +					netdev_info(priv->dev,
> +						    "HOST_SLEEP_ACTIVATE failed: %d\n",
> +						    ret);
>  			}
>  
>  			if (!wait_event_interruptible_timeout(
>  						priv->host_sleep_q,
>  						priv->is_host_sleep_activated,
>  						(10 * HZ))) {
> -				pr_err("host_sleep_q: timer expired\n");
> +				netdev_err(priv->dev,
> +					   "host_sleep_q: timer expired\n");
>  				ret = -1;
>  			}
>  		} else {
> -			pr_err("host sleep: already enabled\n");
> +			netdev_err(priv->dev, "host sleep: already enabled\n");
>  		}
>  	} else {
>  		if (priv->is_host_sleep_activated)
> @@ -1003,7 +1006,8 @@ static void lbs_submit_command(struct lbs_private *priv,
>  	ret = priv->hw_host_to_card(priv, MVMS_CMD, (u8 *) cmd, cmdsize);
>  
>  	if (ret) {
> -		pr_info("DNLD_CMD: hw_host_to_card failed: %d\n", ret);
> +		netdev_info(priv->dev, "DNLD_CMD: hw_host_to_card failed: %d\n",
> +			    ret);
>  		/* Let the timer kick in and retry, and potentially reset
>  		   the whole thing if the condition persists */
>  		timeo = HZ/4;
> @@ -1268,7 +1272,8 @@ int lbs_execute_next_command(struct lbs_private *priv)
>  	spin_lock_irqsave(&priv->driver_lock, flags);
>  
>  	if (priv->cur_cmd) {
> -		pr_alert( "EXEC_NEXT_CMD: already processing command!\n");
> +		netdev_alert(priv->dev,
> +			     "EXEC_NEXT_CMD: already processing command!\n");
>  		spin_unlock_irqrestore(&priv->driver_lock, flags);
>  		ret = -1;
>  		goto done;
> @@ -1430,7 +1435,7 @@ static void lbs_send_confirmsleep(struct lbs_private *priv)
>  	ret = priv->hw_host_to_card(priv, MVMS_CMD, (u8 *) &confirm_sleep,
>  		sizeof(confirm_sleep));
>  	if (ret) {
> -		pr_alert("confirm_sleep failed\n");
> +		netdev_alert(priv->dev, "confirm_sleep failed\n");
>  		goto out;
>  	}
>  
> @@ -1656,7 +1661,8 @@ int __lbs_cmd(struct lbs_private *priv, uint16_t command,
>  	spin_lock_irqsave(&priv->driver_lock, flags);
>  	ret = cmdnode->result;
>  	if (ret)
> -		pr_info("PREP_CMD: command 0x%04x failed: %d\n", command, ret);
> +		netdev_info(priv->dev, "PREP_CMD: command 0x%04x failed: %d\n",
> +			    command, ret);
>  
>  	__lbs_cleanup_and_insert_cmd(priv, cmdnode);
>  	spin_unlock_irqrestore(&priv->driver_lock, flags);
> diff --git a/drivers/net/wireless/libertas/cmdresp.c b/drivers/net/wireless/libertas/cmdresp.c
> index 2cb6f5f..4352e92 100644
> --- a/drivers/net/wireless/libertas/cmdresp.c
> +++ b/drivers/net/wireless/libertas/cmdresp.c
> @@ -3,8 +3,6 @@
>    * responses as well as events generated by firmware.
>    */
>  
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
>  #include <linux/slab.h>
>  #include <linux/delay.h>
>  #include <linux/sched.h>
> @@ -87,17 +85,18 @@ int lbs_process_command_response(struct lbs_private *priv, u8 *data, u32 len)
>  	lbs_deb_hex(LBS_DEB_CMD, "CMD_RESP", (void *) resp, len);
>  
>  	if (resp->seqnum != priv->cur_cmd->cmdbuf->seqnum) {
> -		pr_info("Received CMD_RESP with invalid sequence %d (expected %d)\n",
> -			le16_to_cpu(resp->seqnum),
> -			le16_to_cpu(priv->cur_cmd->cmdbuf->seqnum));
> +		netdev_info(priv->dev,
> +			    "Received CMD_RESP with invalid sequence %d (expected %d)\n",
> +			    le16_to_cpu(resp->seqnum),
> +			    le16_to_cpu(priv->cur_cmd->cmdbuf->seqnum));
>  		spin_unlock_irqrestore(&priv->driver_lock, flags);
>  		ret = -1;
>  		goto done;
>  	}
>  	if (respcmd != CMD_RET(curcmd) &&
>  	    respcmd != CMD_RET_802_11_ASSOCIATE && curcmd != CMD_802_11_ASSOCIATE) {
> -		pr_info("Invalid CMD_RESP %x to command %x!\n",
> -			respcmd, curcmd);
> +		netdev_info(priv->dev, "Invalid CMD_RESP %x to command %x!\n",
> +			    respcmd, curcmd);
>  		spin_unlock_irqrestore(&priv->driver_lock, flags);
>  		ret = -1;
>  		goto done;
> @@ -106,8 +105,9 @@ int lbs_process_command_response(struct lbs_private *priv, u8 *data, u32 len)
>  	if (resp->result == cpu_to_le16(0x0004)) {
>  		/* 0x0004 means -EAGAIN. Drop the response, let it time out
>  		   and be resubmitted */
> -		pr_info("Firmware returns DEFER to command %x. Will let it time out...\n",
> -			le16_to_cpu(resp->command));
> +		netdev_info(priv->dev,
> +			    "Firmware returns DEFER to command %x. Will let it time out...\n",
> +			    le16_to_cpu(resp->command));
>  		spin_unlock_irqrestore(&priv->driver_lock, flags);
>  		ret = -1;
>  		goto done;
> @@ -318,28 +318,28 @@ int lbs_process_event(struct lbs_private *priv, u32 event)
>  		lbs_deb_cmd("EVENT: ADHOC beacon lost\n");
>  		break;
>  	case MACREG_INT_CODE_RSSI_LOW:
> -		pr_alert("EVENT: rssi low\n");
> +		netdev_alert(priv->dev, "EVENT: rssi low\n");
>  		break;
>  	case MACREG_INT_CODE_SNR_LOW:
> -		pr_alert("EVENT: snr low\n");
> +		netdev_alert(priv->dev, "EVENT: snr low\n");
>  		break;
>  	case MACREG_INT_CODE_MAX_FAIL:
> -		pr_alert("EVENT: max fail\n");
> +		netdev_alert(priv->dev, "EVENT: max fail\n");
>  		break;
>  	case MACREG_INT_CODE_RSSI_HIGH:
> -		pr_alert("EVENT: rssi high\n");
> +		netdev_alert(priv->dev, "EVENT: rssi high\n");
>  		break;
>  	case MACREG_INT_CODE_SNR_HIGH:
> -		pr_alert("EVENT: snr high\n");
> +		netdev_alert(priv->dev, "EVENT: snr high\n");
>  		break;
>  
>  	case MACREG_INT_CODE_MESH_AUTO_STARTED:
>  		/* Ignore spurious autostart events */
> -		pr_info("EVENT: MESH_AUTO_STARTED (ignoring)\n");
> +		netdev_info(priv->dev, "EVENT: MESH_AUTO_STARTED (ignoring)\n");
>  		break;
>  
>  	default:
> -		pr_alert("EVENT: unknown event id %d\n", event);
> +		netdev_alert(priv->dev, "EVENT: unknown event id %d\n", event);
>  		break;
>  	}
>  
> diff --git a/drivers/net/wireless/libertas/debugfs.c b/drivers/net/wireless/libertas/debugfs.c
> index c179094..f2f65bc 100644
> --- a/drivers/net/wireless/libertas/debugfs.c
> +++ b/drivers/net/wireless/libertas/debugfs.c
> @@ -1,5 +1,3 @@
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
>  #include <linux/dcache.h>
>  #include <linux/debugfs.h>
>  #include <linux/delay.h>
> @@ -153,13 +151,14 @@ static ssize_t lbs_host_sleep_write(struct file *file,
>  		ret = lbs_set_host_sleep(priv, 0);
>  	else if (host_sleep == 1) {
>  		if (priv->wol_criteria == EHS_REMOVE_WAKEUP) {
> -			pr_info("wake parameters not configured\n");
> +			netdev_info(priv->dev,
> +				    "wake parameters not configured\n");
>  			ret = -EINVAL;
>  			goto out_unlock;
>  		}
>  		ret = lbs_set_host_sleep(priv, 1);
>  	} else {
> -		pr_err("invalid option\n");
> +		netdev_err(priv->dev, "invalid option\n");
>  		ret = -EINVAL;
>  	}
>  
> diff --git a/drivers/net/wireless/libertas/if_cs.c b/drivers/net/wireless/libertas/if_cs.c
> index eb88d9a..d6f757e 100644
> --- a/drivers/net/wireless/libertas/if_cs.c
> +++ b/drivers/net/wireless/libertas/if_cs.c
> @@ -363,7 +363,7 @@ static int if_cs_send_cmd(struct lbs_private *priv, u8 *buf, u16 nb)
>  		if (status & IF_CS_BIT_COMMAND)
>  			break;
>  		if (++loops > 100) {
> -			pr_err("card not ready for commands\n");
> +			netdev_err(priv->dev, "card not ready for commands\n");
>  			goto done;
>  		}
>  		mdelay(1);
> @@ -433,14 +433,16 @@ static int if_cs_receive_cmdres(struct lbs_private *priv, u8 *data, u32 *len)
>  	/* is hardware ready? */
>  	status = if_cs_read16(priv->card, IF_CS_CARD_STATUS);
>  	if ((status & IF_CS_BIT_RESP) == 0) {
> -		pr_err("no cmd response in card\n");
> +		netdev_err(priv->dev, "no cmd response in card\n");
>  		*len = 0;
>  		goto out;
>  	}
>  
>  	*len = if_cs_read16(priv->card, IF_CS_RESP_LEN);
>  	if ((*len == 0) || (*len > LBS_CMD_BUFFER_SIZE)) {
> -		pr_err("card cmd buffer has invalid # of bytes (%d)\n", *len);
> +		netdev_err(priv->dev,
> +			   "card cmd buffer has invalid # of bytes (%d)\n",
> +			   *len);
>  		goto out;
>  	}
>  
> @@ -474,7 +476,9 @@ static struct sk_buff *if_cs_receive_data(struct lbs_private *priv)
>  
>  	len = if_cs_read16(priv->card, IF_CS_READ_LEN);
>  	if (len == 0 || len > MRVDRV_ETH_RX_PACKET_BUFFER_SIZE) {
> -		pr_err("card data buffer has invalid # of bytes (%d)\n", len);
> +		netdev_err(priv->dev,
> +			   "card data buffer has invalid # of bytes (%d)\n",
> +			   len);
>  		priv->dev->stats.rx_dropped++;
>  		goto dat_err;
>  	}
> @@ -752,7 +756,8 @@ static int if_cs_host_to_card(struct lbs_private *priv,
>  		ret = if_cs_send_cmd(priv, buf, nb);
>  		break;
>  	default:
> -		pr_err("%s: unsupported type %d\n", __func__, type);
> +		netdev_err(priv->dev, "%s: unsupported type %d\n",
> +			   __func__, type);
>  	}
>  
>  	lbs_deb_leave_args(LBS_DEB_CS, "ret %d", ret);
> diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
> index ab86779..a7b5cb0 100644
> --- a/drivers/net/wireless/libertas/if_sdio.c
> +++ b/drivers/net/wireless/libertas/if_sdio.c
> @@ -851,7 +851,7 @@ static int if_sdio_enter_deep_sleep(struct lbs_private *priv)
>  	ret = __lbs_cmd(priv, CMD_802_11_DEEP_SLEEP, &cmd, sizeof(cmd),
>  			lbs_cmd_copyback, (unsigned long) &cmd);
>  	if (ret)
> -		pr_err("DEEP_SLEEP cmd failed\n");
> +		netdev_err(priv->dev, "DEEP_SLEEP cmd failed\n");
>  
>  	mdelay(200);
>  	return ret;
> @@ -867,7 +867,7 @@ static int if_sdio_exit_deep_sleep(struct lbs_private *priv)
>  
>  	sdio_writeb(card->func, HOST_POWER_UP, CONFIGURATION_REG, &ret);
>  	if (ret)
> -		pr_err("sdio_writeb failed!\n");
> +		netdev_err(priv->dev, "sdio_writeb failed!\n");
>  
>  	sdio_release_host(card->func);
>  	lbs_deb_leave_args(LBS_DEB_SDIO, "ret %d", ret);
> @@ -884,7 +884,7 @@ static int if_sdio_reset_deep_sleep_wakeup(struct lbs_private *priv)
>  
>  	sdio_writeb(card->func, 0, CONFIGURATION_REG, &ret);
>  	if (ret)
> -		pr_err("sdio_writeb failed!\n");
> +		netdev_err(priv->dev, "sdio_writeb failed!\n");
>  
>  	sdio_release_host(card->func);
>  	lbs_deb_leave_args(LBS_DEB_SDIO, "ret %d", ret);
> @@ -1103,7 +1103,7 @@ static int if_sdio_probe(struct sdio_func *func,
>  		lbs_deb_sdio("send function INIT command\n");
>  		if (__lbs_cmd(priv, CMD_FUNC_INIT, &cmd, sizeof(cmd),
>  				lbs_cmd_copyback, (unsigned long) &cmd))
> -			pr_alert("CMD_FUNC_INIT cmd failed\n");
> +			netdev_alert(priv->dev, "CMD_FUNC_INIT cmd failed\n");
>  	}
>  
>  	ret = lbs_start_card(priv);
> @@ -1204,19 +1204,20 @@ static int if_sdio_suspend(struct device *dev)
>  
>  	mmc_pm_flag_t flags = sdio_get_host_pm_caps(func);
>  
> -	pr_info("%s: suspend: PM flags = 0x%x\n", sdio_func_id(func), flags);
> +	dev_info(dev, "%s: suspend: PM flags = 0x%x\n",
> +		 sdio_func_id(func), flags);
>  
>  	/* If we aren't being asked to wake on anything, we should bail out
>  	 * and let the SD stack power down the card.
>  	 */
>  	if (card->priv->wol_criteria == EHS_REMOVE_WAKEUP) {
> -		pr_info("Suspend without wake params -- powering down card\n");
> +		dev_info(dev, "Suspend without wake params -- powering down card\n");
>  		return -ENOSYS;
>  	}
>  
>  	if (!(flags & MMC_PM_KEEP_POWER)) {
> -		pr_err("%s: cannot remain alive while host is suspended\n",
> -		       sdio_func_id(func));
> +		dev_err(dev, "%s: cannot remain alive while host is suspended\n",
> +			sdio_func_id(func));
>  		return -ENOSYS;
>  	}
>  
> @@ -1237,7 +1238,7 @@ static int if_sdio_resume(struct device *dev)
>  	struct if_sdio_card *card = sdio_get_drvdata(func);
>  	int ret;
>  
> -	pr_info("%s: resume: we're back\n", sdio_func_id(func));
> +	dev_info(dev, "%s: resume: we're back\n", sdio_func_id(func));
>  
>  	ret = lbs_resume(card->priv);
>  
> diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
> index bebd9f0..30f21b1 100644
> --- a/drivers/net/wireless/libertas/if_spi.c
> +++ b/drivers/net/wireless/libertas/if_spi.c
> @@ -559,6 +559,7 @@ static int if_spi_prog_main_firmware_check_len(struct if_spi_card *card,
>  static int if_spi_prog_main_firmware(struct if_spi_card *card,
>  					const struct firmware *firmware)
>  {
> +	struct lbs_private *priv = card->priv;
>  	int len, prev_len;
>  	int bytes, crc_err = 0, err = 0;
>  	const u8 *fw;
> @@ -572,8 +573,9 @@ static int if_spi_prog_main_firmware(struct if_spi_card *card,
>  
>  	err = spu_wait_for_u16(card, IF_SPI_SCRATCH_1_REG, 0, 0);
>  	if (err) {
> -		pr_err("%s: timed out waiting for initial scratch reg = 0\n",
> -		       __func__);
> +		netdev_err(priv->dev,
> +			   "%s: timed out waiting for initial scratch reg = 0\n",
> +			   __func__);
>  		goto out;
>  	}
>  
> @@ -589,7 +591,8 @@ static int if_spi_prog_main_firmware(struct if_spi_card *card,
>  		if (bytes < 0) {
>  			/* If there are no more bytes left, we would normally
>  			 * expect to have terminated with len = 0 */
> -			pr_err("Firmware load wants more bytes than we have to offer\n");
> +			netdev_err(priv->dev,
> +				   "Firmware load wants more bytes than we have to offer\n");
>  			break;
>  		}
>  		if (crc_err) {
> @@ -674,12 +677,14 @@ static int if_spi_c2h_cmd(struct if_spi_card *card)
>  	if (err)
>  		goto out;
>  	if (!len) {
> -		pr_err("%s: error: card has no data for host\n", __func__);
> +		netdev_err(priv->dev, "%s: error: card has no data for host\n",
> +			   __func__);
>  		err = -EINVAL;
>  		goto out;
>  	} else if (len > IF_SPI_CMD_BUF_SIZE) {
> -		pr_err("%s: error: response packet too large: %d bytes, but maximum is %d\n",
> -		       __func__, len, IF_SPI_CMD_BUF_SIZE);
> +		netdev_err(priv->dev,
> +			   "%s: error: response packet too large: %d bytes, but maximum is %d\n",
> +			   __func__, len, IF_SPI_CMD_BUF_SIZE);
>  		err = -EINVAL;
>  		goto out;
>  	}
> @@ -700,7 +705,7 @@ static int if_spi_c2h_cmd(struct if_spi_card *card)
>  
>  out:
>  	if (err)
> -		pr_err("%s: err=%d\n", __func__, err);
> +		netdev_err(priv->dev, "%s: err=%d\n", __func__, err);
>  	lbs_deb_leave(LBS_DEB_SPI);
>  	return err;
>  }
> @@ -708,6 +713,7 @@ out:
>  /* Move data from the card to the host */
>  static int if_spi_c2h_data(struct if_spi_card *card)
>  {
> +	struct lbs_private *priv = card->priv;
>  	struct sk_buff *skb;
>  	char *data;
>  	u16 len;
> @@ -720,12 +726,14 @@ static int if_spi_c2h_data(struct if_spi_card *card)
>  	if (err)
>  		goto out;
>  	if (!len) {
> -		pr_err("%s: error: card has no data for host\n", __func__);
> +		netdev_err(priv->dev, "%s: error: card has no data for host\n",
> +			   __func__);
>  		err = -EINVAL;
>  		goto out;
>  	} else if (len > MRVDRV_ETH_RX_PACKET_BUFFER_SIZE) {
> -		pr_err("%s: error: card has %d bytes of data, but our maximum skb size is %zu\n",
> -		       __func__, len, MRVDRV_ETH_RX_PACKET_BUFFER_SIZE);
> +		netdev_err(priv->dev,
> +			   "%s: error: card has %d bytes of data, but our maximum skb size is %zu\n",
> +			   __func__, len, MRVDRV_ETH_RX_PACKET_BUFFER_SIZE);
>  		err = -EINVAL;
>  		goto out;
>  	}
> @@ -756,7 +764,7 @@ free_skb:
>  	dev_kfree_skb(skb);
>  out:
>  	if (err)
> -		pr_err("%s: err=%d\n", __func__, err);
> +		netdev_err(priv->dev, "%s: err=%d\n", __func__, err);
>  	lbs_deb_leave(LBS_DEB_SPI);
>  	return err;
>  }
> @@ -765,6 +773,7 @@ out:
>  static void if_spi_h2c(struct if_spi_card *card,
>  			struct if_spi_packet *packet, int type)
>  {
> +	struct lbs_private *priv = card->priv;
>  	int err = 0;
>  	u16 int_type, port_reg;
>  
> @@ -778,7 +787,8 @@ static void if_spi_h2c(struct if_spi_card *card,
>  		port_reg = IF_SPI_CMD_RDWRPORT_REG;
>  		break;
>  	default:
> -		pr_err("can't transfer buffer of type %d\n", type);
> +		netdev_err(priv->dev, "can't transfer buffer of type %d\n",
> +			   type);
>  		err = -EINVAL;
>  		goto out;
>  	}
> @@ -792,7 +802,7 @@ out:
>  	kfree(packet);
>  
>  	if (err)
> -		pr_err("%s: error %d\n", __func__, err);
> +		netdev_err(priv->dev, "%s: error %d\n", __func__, err);
>  }
>  
>  /* Inform the host about a card event */
> @@ -816,7 +826,7 @@ static void if_spi_e2h(struct if_spi_card *card)
>  	lbs_queue_event(priv, cause & 0xff);
>  out:
>  	if (err)
> -		pr_err("%s: error %d\n", __func__, err);
> +		netdev_err(priv->dev, "%s: error %d\n", __func__, err);
>  }
>  
>  static void if_spi_host_to_card_worker(struct work_struct *work)
> @@ -826,8 +836,10 @@ static void if_spi_host_to_card_worker(struct work_struct *work)
>  	u16 hiStatus;
>  	unsigned long flags;
>  	struct if_spi_packet *packet;
> +	struct lbs_private *priv;
>  
>  	card = container_of(work, struct if_spi_card, packet_work);
> +	priv = card->priv;
>  
>  	lbs_deb_enter(LBS_DEB_SPI);
>  
> @@ -836,7 +848,7 @@ static void if_spi_host_to_card_worker(struct work_struct *work)
>  	err = spu_read_u16(card, IF_SPI_HOST_INT_STATUS_REG,
>  				&hiStatus);
>  	if (err) {
> -		pr_err("I/O error\n");
> +		netdev_err(priv->dev, "I/O error\n");
>  		goto err;
>  	}
>  
> @@ -898,7 +910,7 @@ static void if_spi_host_to_card_worker(struct work_struct *work)
>  
>  err:
>  	if (err)
> -		pr_err("%s: got error %d\n", __func__, err);
> +		netdev_err(priv->dev, "%s: got error %d\n", __func__, err);
>  
>  	lbs_deb_leave(LBS_DEB_SPI);
>  }
> @@ -920,7 +932,8 @@ static int if_spi_host_to_card(struct lbs_private *priv,
>  	lbs_deb_enter_args(LBS_DEB_SPI, "type %d, bytes %d", type, nb);
>  
>  	if (nb == 0) {
> -		pr_err("%s: invalid size requested: %d\n", __func__, nb);
> +		netdev_err(priv->dev, "%s: invalid size requested: %d\n",
> +			   __func__, nb);
>  		err = -EINVAL;
>  		goto out;
>  	}
> @@ -948,7 +961,8 @@ static int if_spi_host_to_card(struct lbs_private *priv,
>  		spin_unlock_irqrestore(&card->buffer_lock, flags);
>  		break;
>  	default:
> -		pr_err("can't transfer buffer of type %d\n", type);
> +		netdev_err(priv->dev, "can't transfer buffer of type %d\n",
> +			   type);
>  		err = -EINVAL;
>  		break;
>  	}
> @@ -981,6 +995,7 @@ static irqreturn_t if_spi_host_interrupt(int irq, void *dev_id)
>  
>  static int if_spi_init_card(struct if_spi_card *card)
>  {
> +	struct lbs_private *priv = card->priv;
>  	struct spi_device *spi = card->spi;
>  	int err, i;
>  	u32 scratch;
> @@ -1009,7 +1024,8 @@ static int if_spi_init_card(struct if_spi_card *card)
>  				break;
>  		}
>  		if (i == ARRAY_SIZE(fw_table)) {
> -			pr_err("Unsupported chip_id: 0x%02x\n", card->card_id);
> +			netdev_err(priv->dev, "Unsupported chip_id: 0x%02x\n",
> +				   card->card_id);
>  			err = -ENODEV;
>  			goto out;
>  		}
> @@ -1018,7 +1034,8 @@ static int if_spi_init_card(struct if_spi_card *card)
>  					card->card_id, &fw_table[0], &helper,
>  					&mainfw);
>  		if (err) {
> -			pr_err("failed to find firmware (%d)\n", err);
> +			netdev_err(priv->dev, "failed to find firmware (%d)\n",
> +				   err);
>  			goto out;
>  		}
>  
> diff --git a/drivers/net/wireless/libertas/if_usb.c b/drivers/net/wireless/libertas/if_usb.c
> index 7260791..63e7e2c 100644
> --- a/drivers/net/wireless/libertas/if_usb.c
> +++ b/drivers/net/wireless/libertas/if_usb.c
> @@ -198,7 +198,7 @@ static void if_usb_setup_firmware(struct lbs_private *priv)
>  	wake_method.hdr.size = cpu_to_le16(sizeof(wake_method));
>  	wake_method.action = cpu_to_le16(CMD_ACT_GET);
>  	if (lbs_cmd_with_response(priv, CMD_802_11_FW_WAKE_METHOD, &wake_method)) {
> -		pr_info("Firmware does not seem to support PS mode\n");
> +		netdev_info(priv->dev, "Firmware does not seem to support PS mode\n");
>  		priv->fwcapinfo &= ~FW_CAPINFO_PS;
>  	} else {
>  		if (le16_to_cpu(wake_method.method) == CMD_WAKE_METHOD_COMMAND_INT) {
> @@ -207,7 +207,8 @@ static void if_usb_setup_firmware(struct lbs_private *priv)
>  			/* The versions which boot up this way don't seem to
>  			   work even if we set it to the command interrupt */
>  			priv->fwcapinfo &= ~FW_CAPINFO_PS;
> -			pr_info("Firmware doesn't wake via command interrupt; disabling PS mode\n");
> +			netdev_info(priv->dev,
> +				    "Firmware doesn't wake via command interrupt; disabling PS mode\n");
>  		}
>  	}
>  }
> @@ -343,10 +344,12 @@ static int if_usb_probe(struct usb_interface *intf,
>  	usb_set_intfdata(intf, cardp);
>  
>  	if (device_create_file(&priv->dev->dev, &dev_attr_lbs_flash_fw))
> -		pr_err("cannot register lbs_flash_fw attribute\n");
> +		netdev_err(priv->dev,
> +			   "cannot register lbs_flash_fw attribute\n");
>  
>  	if (device_create_file(&priv->dev->dev, &dev_attr_lbs_flash_boot2))
> -		pr_err("cannot register lbs_flash_boot2 attribute\n");
> +		netdev_err(priv->dev,
> +			   "cannot register lbs_flash_boot2 attribute\n");
>  
>  	/*
>  	 * EHS_REMOVE_WAKEUP is not supported on all versions of the firmware.
> diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
> index 639461b..84ffafe 100644
> --- a/drivers/net/wireless/libertas/main.c
> +++ b/drivers/net/wireless/libertas/main.c
> @@ -155,7 +155,7 @@ static void lbs_tx_timeout(struct net_device *dev)
>  
>  	lbs_deb_enter(LBS_DEB_TX);
>  
> -	pr_err("tx watch dog timeout\n");
> +	netdev_err(dev, "tx watch dog timeout\n");
>  
>  	dev->trans_start = jiffies; /* prevent tx timeout */
>  
> @@ -464,8 +464,8 @@ static int lbs_thread(void *data)
>  		if (priv->cmd_timed_out && priv->cur_cmd) {
>  			struct cmd_ctrl_node *cmdnode = priv->cur_cmd;
>  
> -			pr_info("Timeout submitting command 0x%04x\n",
> -				le16_to_cpu(cmdnode->cmdbuf->command));
> +			netdev_info(dev, "Timeout submitting command 0x%04x\n",
> +				    le16_to_cpu(cmdnode->cmdbuf->command));
>  			lbs_complete_command(priv, cmdnode, -ETIMEDOUT);
>  			if (priv->reset_card)
>  				priv->reset_card(priv);
> @@ -492,7 +492,8 @@ static int lbs_thread(void *data)
>  				 * after firmware fixes it
>  				 */
>  				priv->psstate = PS_STATE_AWAKE;
> -				pr_alert("ignore PS_SleepConfirm in non-connected state\n");
> +				netdev_alert(dev,
> +					     "ignore PS_SleepConfirm in non-connected state\n");
>  			}
>  		}
>  
> @@ -586,7 +587,8 @@ int lbs_suspend(struct lbs_private *priv)
>  	if (priv->is_deep_sleep) {
>  		ret = lbs_set_deep_sleep(priv, 0);
>  		if (ret) {
> -			pr_err("deep sleep cancellation failed: %d\n", ret);
> +			netdev_err(priv->dev,
> +				   "deep sleep cancellation failed: %d\n", ret);
>  			return ret;
>  		}
>  		priv->deep_sleep_required = 1;
> @@ -619,7 +621,8 @@ int lbs_resume(struct lbs_private *priv)
>  		priv->deep_sleep_required = 0;
>  		ret = lbs_set_deep_sleep(priv, 1);
>  		if (ret)
> -			pr_err("deep sleep activation failed: %d\n", ret);
> +			netdev_err(priv->dev,
> +				   "deep sleep activation failed: %d\n", ret);
>  	}
>  
>  	if (priv->setup_fw_on_resume)
> @@ -645,8 +648,8 @@ static void lbs_cmd_timeout_handler(unsigned long data)
>  	if (!priv->cur_cmd)
>  		goto out;
>  
> -	pr_info("command 0x%04x timed out\n",
> -		le16_to_cpu(priv->cur_cmd->cmdbuf->command));
> +	netdev_info(priv->dev, "command 0x%04x timed out\n",
> +		    le16_to_cpu(priv->cur_cmd->cmdbuf->command));
>  
>  	priv->cmd_timed_out = 1;
>  	wake_up_interruptible(&priv->waitq);
> @@ -961,7 +964,7 @@ int lbs_start_card(struct lbs_private *priv)
>  
>  	lbs_debugfs_init_one(priv, dev);
>  
> -	pr_info("%s: Marvell WLAN 802.11 adapter\n", dev->name);
> +	netdev_info(dev, "Marvell WLAN 802.11 adapter\n");
>  
>  	ret = 0;
>  
> @@ -1088,14 +1091,16 @@ int lbs_get_firmware(struct device *dev, const char *user_helper,
>  	if (user_helper) {
>  		ret = request_firmware(helper, user_helper, dev);
>  		if (ret) {
> -			pr_err("couldn't find helper firmware %s", user_helper);
> +			dev_err(dev, "couldn't find helper firmware %s\n",
> +				user_helper);
>  			goto fail;
>  		}
>  	}
>  	if (user_mainfw) {
>  		ret = request_firmware(mainfw, user_mainfw, dev);
>  		if (ret) {
> -			pr_err("couldn't find main firmware %s", user_mainfw);
> +			dev_err(dev, "couldn't find main firmware %s\n",
> +				user_mainfw);
>  			goto fail;
>  		}
>  	}
> diff --git a/drivers/net/wireless/libertas/mesh.c b/drivers/net/wireless/libertas/mesh.c
> index f7c51cb..f4c4f1c 100644
> --- a/drivers/net/wireless/libertas/mesh.c
> +++ b/drivers/net/wireless/libertas/mesh.c
> @@ -248,7 +248,7 @@ int lbs_init_mesh(struct lbs_private *priv)
>  		lbs_add_mesh(priv);
>  
>  		if (device_create_file(&dev->dev, &dev_attr_lbs_mesh))
> -			pr_err("cannot register lbs_mesh attribute\n");
> +			netdev_err(dev, "cannot register lbs_mesh attribute\n");
>  
>  		ret = 1;
>  	}
> @@ -928,7 +928,7 @@ static ssize_t mesh_id_get(struct device *dev, struct device_attribute *attr,
>  		return ret;
>  
>  	if (defs.meshie.val.mesh_id_len > IEEE80211_MAX_SSID_LEN) {
> -		pr_err("inconsistent mesh ID length\n");
> +		dev_err(dev, "inconsistent mesh ID length\n");
>  		defs.meshie.val.mesh_id_len = IEEE80211_MAX_SSID_LEN;
>  	}
>  
> diff --git a/drivers/net/wireless/libertas/rx.c b/drivers/net/wireless/libertas/rx.c
> index fd045aa..f2ef0ae 100644
> --- a/drivers/net/wireless/libertas/rx.c
> +++ b/drivers/net/wireless/libertas/rx.c
> @@ -251,7 +251,7 @@ static int process_rxed_802_11_packet(struct lbs_private *priv,
>  	/* add space for the new radio header */
>  	if ((skb_headroom(skb) < sizeof(struct rx_radiotap_hdr)) &&
>  	    pskb_expand_head(skb, sizeof(struct rx_radiotap_hdr), 0, GFP_ATOMIC)) {
> -		pr_alert("%s: couldn't pskb_expand_head\n", __func__);
> +		netdev_alert(dev, "%s: couldn't pskb_expand_head\n", __func__);
>  		ret = -ENOMEM;
>  		kfree_skb(skb);
>  		goto done;

^ permalink raw reply

* Re: [PATCH 1/2] libertas: Convert lbs_pr_<level> to pr_<level>
From: Dan Williams @ 2011-05-04 22:15 UTC (permalink / raw)
  To: Joe Perches
  Cc: John W. Linville, libertas-dev, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <353dfb1607172b5a3dbaccddbe52c24312d42af4.1304379925.git.joe@perches.com>

On Mon, 2011-05-02 at 16:49 -0700, Joe Perches wrote:
> Use the standard pr_<level> functions eases grep a bit.
> 
> Added a few missing terminating newlines to messages.
> Coalesced long formats.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Acked-by: Dan Williams <dcbw@redhat.com>

> ---
>  drivers/net/wireless/libertas/cfg.c     |   15 +++---
>  drivers/net/wireless/libertas/cmd.c     |   36 +++++++-------
>  drivers/net/wireless/libertas/cmdresp.c |   29 +++++++-----
>  drivers/net/wireless/libertas/debugfs.c |    6 ++-
>  drivers/net/wireless/libertas/defs.h    |    7 ---
>  drivers/net/wireless/libertas/if_cs.c   |   52 +++++++++++----------
>  drivers/net/wireless/libertas/if_sdio.c |   38 ++++++++--------
>  drivers/net/wireless/libertas/if_spi.c  |   74 ++++++++++++++-----------------
>  drivers/net/wireless/libertas/if_usb.c  |   41 +++++++++--------
>  drivers/net/wireless/libertas/main.c    |   33 +++++++-------
>  drivers/net/wireless/libertas/mesh.c    |    8 ++-
>  drivers/net/wireless/libertas/rx.c      |    7 ++-
>  12 files changed, 174 insertions(+), 172 deletions(-)
> 
> diff --git a/drivers/net/wireless/libertas/cfg.c b/drivers/net/wireless/libertas/cfg.c
> index 5caa2ac..d6e0656 100644
> --- a/drivers/net/wireless/libertas/cfg.c
> +++ b/drivers/net/wireless/libertas/cfg.c
> @@ -6,6 +6,8 @@
>   *
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/sched.h>
>  #include <linux/wait.h>
>  #include <linux/slab.h>
> @@ -1314,8 +1316,7 @@ static int lbs_cfg_connect(struct wiphy *wiphy, struct net_device *dev,
>  		sme->ssid, sme->ssid_len,
>  		WLAN_CAPABILITY_ESS, WLAN_CAPABILITY_ESS);
>  	if (!bss) {
> -		lbs_pr_err("assoc: bss %pM not in scan results\n",
> -			   sme->bssid);
> +		pr_err("assoc: bss %pM not in scan results\n", sme->bssid);
>  		ret = -ENOENT;
>  		goto done;
>  	}
> @@ -1372,8 +1373,8 @@ static int lbs_cfg_connect(struct wiphy *wiphy, struct net_device *dev,
>  		lbs_enable_rsn(priv, sme->crypto.cipher_group != 0);
>  		break;
>  	default:
> -		lbs_pr_err("unsupported cipher group 0x%x\n",
> -			   sme->crypto.cipher_group);
> +		pr_err("unsupported cipher group 0x%x\n",
> +		       sme->crypto.cipher_group);
>  		ret = -ENOTSUPP;
>  		goto done;
>  	}
> @@ -1491,7 +1492,7 @@ static int lbs_cfg_add_key(struct wiphy *wiphy, struct net_device *netdev,
>  				     params->key, params->key_len);
>  		break;
>  	default:
> -		lbs_pr_err("unhandled cipher 0x%x\n", params->cipher);
> +		pr_err("unhandled cipher 0x%x\n", params->cipher);
>  		ret = -ENOTSUPP;
>  		break;
>  	}
> @@ -2118,13 +2119,13 @@ int lbs_cfg_register(struct lbs_private *priv)
>  
>  	ret = wiphy_register(wdev->wiphy);
>  	if (ret < 0)
> -		lbs_pr_err("cannot register wiphy device\n");
> +		pr_err("cannot register wiphy device\n");
>  
>  	priv->wiphy_registered = true;
>  
>  	ret = register_netdev(priv->dev);
>  	if (ret)
> -		lbs_pr_err("cannot register network device\n");
> +		pr_err("cannot register network device\n");
>  
>  	INIT_DELAYED_WORK(&priv->scan_work, lbs_scan_worker);
>  
> diff --git a/drivers/net/wireless/libertas/cmd.c b/drivers/net/wireless/libertas/cmd.c
> index 7e8a658..10ca485 100644
> --- a/drivers/net/wireless/libertas/cmd.c
> +++ b/drivers/net/wireless/libertas/cmd.c
> @@ -3,6 +3,8 @@
>    * It prepares command and sends it to firmware when it is ready.
>    */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/kfifo.h>
>  #include <linux/sched.h>
>  #include <linux/slab.h>
> @@ -108,7 +110,7 @@ int lbs_update_hw_spec(struct lbs_private *priv)
>  	 * CF card    firmware 5.0.16p0:   cap 0x00000303
>  	 * USB dongle firmware 5.110.17p2: cap 0x00000303
>  	 */
> -	lbs_pr_info("%pM, fw %u.%u.%up%u, cap 0x%08x\n",
> +	pr_info("%pM, fw %u.%u.%up%u, cap 0x%08x\n",
>  		cmd.permanentaddr,
>  		priv->fwrelease >> 24 & 0xff,
>  		priv->fwrelease >> 16 & 0xff,
> @@ -139,7 +141,7 @@ int lbs_update_hw_spec(struct lbs_private *priv)
>  	/* if it's unidentified region code, use the default (USA) */
>  	if (i >= MRVDRV_MAX_REGION_CODE) {
>  		priv->regioncode = 0x10;
> -		lbs_pr_info("unidentified region code; using the default (USA)\n");
> +		pr_info("unidentified region code; using the default (USA)\n");
>  	}
>  
>  	if (priv->current_addr[0] == 0xff)
> @@ -209,7 +211,7 @@ int lbs_host_sleep_cfg(struct lbs_private *priv, uint32_t criteria,
>  					(uint8_t *)&cmd_config.wol_conf,
>  					sizeof(struct wol_config));
>  	} else {
> -		lbs_pr_info("HOST_SLEEP_CFG failed %d\n", ret);
> +		pr_info("HOST_SLEEP_CFG failed %d\n", ret);
>  	}
>  
>  	return ret;
> @@ -312,7 +314,7 @@ static int lbs_wait_for_ds_awake(struct lbs_private *priv)
>  	if (priv->is_deep_sleep) {
>  		if (!wait_event_interruptible_timeout(priv->ds_awake_q,
>  					!priv->is_deep_sleep, (10 * HZ))) {
> -			lbs_pr_err("ds_awake_q: timer expired\n");
> +			pr_err("ds_awake_q: timer expired\n");
>  			ret = -1;
>  		}
>  	}
> @@ -337,7 +339,7 @@ int lbs_set_deep_sleep(struct lbs_private *priv, int deep_sleep)
>  				netif_carrier_off(priv->dev);
>  			}
>  		} else {
> -			lbs_pr_err("deep sleep: already enabled\n");
> +			pr_err("deep sleep: already enabled\n");
>  		}
>  	} else {
>  		if (priv->is_deep_sleep) {
> @@ -347,8 +349,7 @@ int lbs_set_deep_sleep(struct lbs_private *priv, int deep_sleep)
>  			if (!ret) {
>  				ret = lbs_wait_for_ds_awake(priv);
>  				if (ret)
> -					lbs_pr_err("deep sleep: wakeup"
> -							"failed\n");
> +					pr_err("deep sleep: wakeup failed\n");
>  			}
>  		}
>  	}
> @@ -382,8 +383,8 @@ int lbs_set_host_sleep(struct lbs_private *priv, int host_sleep)
>  			ret = lbs_host_sleep_cfg(priv, priv->wol_criteria,
>  					(struct wol_config *)NULL);
>  			if (ret) {
> -				lbs_pr_info("Host sleep configuration failed: "
> -						"%d\n", ret);
> +				pr_info("Host sleep configuration failed: %d\n",
> +					ret);
>  				return ret;
>  			}
>  			if (priv->psstate == PS_STATE_FULL_POWER) {
> @@ -393,19 +394,19 @@ int lbs_set_host_sleep(struct lbs_private *priv, int host_sleep)
>  						sizeof(cmd),
>  						lbs_ret_host_sleep_activate, 0);
>  				if (ret)
> -					lbs_pr_info("HOST_SLEEP_ACTIVATE "
> -							"failed: %d\n", ret);
> +					pr_info("HOST_SLEEP_ACTIVATE failed: %d\n",
> +						ret);
>  			}
>  
>  			if (!wait_event_interruptible_timeout(
>  						priv->host_sleep_q,
>  						priv->is_host_sleep_activated,
>  						(10 * HZ))) {
> -				lbs_pr_err("host_sleep_q: timer expired\n");
> +				pr_err("host_sleep_q: timer expired\n");
>  				ret = -1;
>  			}
>  		} else {
> -			lbs_pr_err("host sleep: already enabled\n");
> +			pr_err("host sleep: already enabled\n");
>  		}
>  	} else {
>  		if (priv->is_host_sleep_activated)
> @@ -1002,7 +1003,7 @@ static void lbs_submit_command(struct lbs_private *priv,
>  	ret = priv->hw_host_to_card(priv, MVMS_CMD, (u8 *) cmd, cmdsize);
>  
>  	if (ret) {
> -		lbs_pr_info("DNLD_CMD: hw_host_to_card failed: %d\n", ret);
> +		pr_info("DNLD_CMD: hw_host_to_card failed: %d\n", ret);
>  		/* Let the timer kick in and retry, and potentially reset
>  		   the whole thing if the condition persists */
>  		timeo = HZ/4;
> @@ -1267,7 +1268,7 @@ int lbs_execute_next_command(struct lbs_private *priv)
>  	spin_lock_irqsave(&priv->driver_lock, flags);
>  
>  	if (priv->cur_cmd) {
> -		lbs_pr_alert( "EXEC_NEXT_CMD: already processing command!\n");
> +		pr_alert( "EXEC_NEXT_CMD: already processing command!\n");
>  		spin_unlock_irqrestore(&priv->driver_lock, flags);
>  		ret = -1;
>  		goto done;
> @@ -1429,7 +1430,7 @@ static void lbs_send_confirmsleep(struct lbs_private *priv)
>  	ret = priv->hw_host_to_card(priv, MVMS_CMD, (u8 *) &confirm_sleep,
>  		sizeof(confirm_sleep));
>  	if (ret) {
> -		lbs_pr_alert("confirm_sleep failed\n");
> +		pr_alert("confirm_sleep failed\n");
>  		goto out;
>  	}
>  
> @@ -1655,8 +1656,7 @@ int __lbs_cmd(struct lbs_private *priv, uint16_t command,
>  	spin_lock_irqsave(&priv->driver_lock, flags);
>  	ret = cmdnode->result;
>  	if (ret)
> -		lbs_pr_info("PREP_CMD: command 0x%04x failed: %d\n",
> -			    command, ret);
> +		pr_info("PREP_CMD: command 0x%04x failed: %d\n", command, ret);
>  
>  	__lbs_cleanup_and_insert_cmd(priv, cmdnode);
>  	spin_unlock_irqrestore(&priv->driver_lock, flags);
> diff --git a/drivers/net/wireless/libertas/cmdresp.c b/drivers/net/wireless/libertas/cmdresp.c
> index 5e95da9..2cb6f5f 100644
> --- a/drivers/net/wireless/libertas/cmdresp.c
> +++ b/drivers/net/wireless/libertas/cmdresp.c
> @@ -2,6 +2,9 @@
>    * This file contains the handling of command
>    * responses as well as events generated by firmware.
>    */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/slab.h>
>  #include <linux/delay.h>
>  #include <linux/sched.h>
> @@ -84,15 +87,17 @@ int lbs_process_command_response(struct lbs_private *priv, u8 *data, u32 len)
>  	lbs_deb_hex(LBS_DEB_CMD, "CMD_RESP", (void *) resp, len);
>  
>  	if (resp->seqnum != priv->cur_cmd->cmdbuf->seqnum) {
> -		lbs_pr_info("Received CMD_RESP with invalid sequence %d (expected %d)\n",
> -			    le16_to_cpu(resp->seqnum), le16_to_cpu(priv->cur_cmd->cmdbuf->seqnum));
> +		pr_info("Received CMD_RESP with invalid sequence %d (expected %d)\n",
> +			le16_to_cpu(resp->seqnum),
> +			le16_to_cpu(priv->cur_cmd->cmdbuf->seqnum));
>  		spin_unlock_irqrestore(&priv->driver_lock, flags);
>  		ret = -1;
>  		goto done;
>  	}
>  	if (respcmd != CMD_RET(curcmd) &&
>  	    respcmd != CMD_RET_802_11_ASSOCIATE && curcmd != CMD_802_11_ASSOCIATE) {
> -		lbs_pr_info("Invalid CMD_RESP %x to command %x!\n", respcmd, curcmd);
> +		pr_info("Invalid CMD_RESP %x to command %x!\n",
> +			respcmd, curcmd);
>  		spin_unlock_irqrestore(&priv->driver_lock, flags);
>  		ret = -1;
>  		goto done;
> @@ -101,8 +106,8 @@ int lbs_process_command_response(struct lbs_private *priv, u8 *data, u32 len)
>  	if (resp->result == cpu_to_le16(0x0004)) {
>  		/* 0x0004 means -EAGAIN. Drop the response, let it time out
>  		   and be resubmitted */
> -		lbs_pr_info("Firmware returns DEFER to command %x. Will let it time out...\n",
> -			    le16_to_cpu(resp->command));
> +		pr_info("Firmware returns DEFER to command %x. Will let it time out...\n",
> +			le16_to_cpu(resp->command));
>  		spin_unlock_irqrestore(&priv->driver_lock, flags);
>  		ret = -1;
>  		goto done;
> @@ -313,28 +318,28 @@ int lbs_process_event(struct lbs_private *priv, u32 event)
>  		lbs_deb_cmd("EVENT: ADHOC beacon lost\n");
>  		break;
>  	case MACREG_INT_CODE_RSSI_LOW:
> -		lbs_pr_alert("EVENT: rssi low\n");
> +		pr_alert("EVENT: rssi low\n");
>  		break;
>  	case MACREG_INT_CODE_SNR_LOW:
> -		lbs_pr_alert("EVENT: snr low\n");
> +		pr_alert("EVENT: snr low\n");
>  		break;
>  	case MACREG_INT_CODE_MAX_FAIL:
> -		lbs_pr_alert("EVENT: max fail\n");
> +		pr_alert("EVENT: max fail\n");
>  		break;
>  	case MACREG_INT_CODE_RSSI_HIGH:
> -		lbs_pr_alert("EVENT: rssi high\n");
> +		pr_alert("EVENT: rssi high\n");
>  		break;
>  	case MACREG_INT_CODE_SNR_HIGH:
> -		lbs_pr_alert("EVENT: snr high\n");
> +		pr_alert("EVENT: snr high\n");
>  		break;
>  
>  	case MACREG_INT_CODE_MESH_AUTO_STARTED:
>  		/* Ignore spurious autostart events */
> -		lbs_pr_info("EVENT: MESH_AUTO_STARTED (ignoring)\n");
> +		pr_info("EVENT: MESH_AUTO_STARTED (ignoring)\n");
>  		break;
>  
>  	default:
> -		lbs_pr_alert("EVENT: unknown event id %d\n", event);
> +		pr_alert("EVENT: unknown event id %d\n", event);
>  		break;
>  	}
>  
> diff --git a/drivers/net/wireless/libertas/debugfs.c b/drivers/net/wireless/libertas/debugfs.c
> index fbf3b033..c179094 100644
> --- a/drivers/net/wireless/libertas/debugfs.c
> +++ b/drivers/net/wireless/libertas/debugfs.c
> @@ -1,3 +1,5 @@
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/dcache.h>
>  #include <linux/debugfs.h>
>  #include <linux/delay.h>
> @@ -151,13 +153,13 @@ static ssize_t lbs_host_sleep_write(struct file *file,
>  		ret = lbs_set_host_sleep(priv, 0);
>  	else if (host_sleep == 1) {
>  		if (priv->wol_criteria == EHS_REMOVE_WAKEUP) {
> -			lbs_pr_info("wake parameters not configured");
> +			pr_info("wake parameters not configured\n");
>  			ret = -EINVAL;
>  			goto out_unlock;
>  		}
>  		ret = lbs_set_host_sleep(priv, 1);
>  	} else {
> -		lbs_pr_err("invalid option\n");
> +		pr_err("invalid option\n");
>  		ret = -EINVAL;
>  	}
>  
> diff --git a/drivers/net/wireless/libertas/defs.h b/drivers/net/wireless/libertas/defs.h
> index d00c728..c9b89b0 100644
> --- a/drivers/net/wireless/libertas/defs.h
> +++ b/drivers/net/wireless/libertas/defs.h
> @@ -89,13 +89,6 @@ do { if ((lbs_debug & (grp)) == (grp)) \
>  #define lbs_deb_spi(fmt, args...)       LBS_DEB_LL(LBS_DEB_SPI, " spi", fmt, ##args)
>  #define lbs_deb_cfg80211(fmt, args...)  LBS_DEB_LL(LBS_DEB_CFG80211, " cfg80211", fmt, ##args)
>  
> -#define lbs_pr_info(format, args...) \
> -	printk(KERN_INFO DRV_NAME": " format, ## args)
> -#define lbs_pr_err(format, args...) \
> -	printk(KERN_ERR DRV_NAME": " format, ## args)
> -#define lbs_pr_alert(format, args...) \
> -	printk(KERN_ALERT DRV_NAME": " format, ## args)
> -
>  #ifdef DEBUG
>  static inline void lbs_deb_hex(unsigned int grp, const char *prompt, u8 *buf, int len)
>  {
> diff --git a/drivers/net/wireless/libertas/if_cs.c b/drivers/net/wireless/libertas/if_cs.c
> index 8712cb2..eb88d9a 100644
> --- a/drivers/net/wireless/libertas/if_cs.c
> +++ b/drivers/net/wireless/libertas/if_cs.c
> @@ -21,6 +21,8 @@
>  
>  */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <linux/delay.h>
> @@ -361,7 +363,7 @@ static int if_cs_send_cmd(struct lbs_private *priv, u8 *buf, u16 nb)
>  		if (status & IF_CS_BIT_COMMAND)
>  			break;
>  		if (++loops > 100) {
> -			lbs_pr_err("card not ready for commands\n");
> +			pr_err("card not ready for commands\n");
>  			goto done;
>  		}
>  		mdelay(1);
> @@ -431,14 +433,14 @@ static int if_cs_receive_cmdres(struct lbs_private *priv, u8 *data, u32 *len)
>  	/* is hardware ready? */
>  	status = if_cs_read16(priv->card, IF_CS_CARD_STATUS);
>  	if ((status & IF_CS_BIT_RESP) == 0) {
> -		lbs_pr_err("no cmd response in card\n");
> +		pr_err("no cmd response in card\n");
>  		*len = 0;
>  		goto out;
>  	}
>  
>  	*len = if_cs_read16(priv->card, IF_CS_RESP_LEN);
>  	if ((*len == 0) || (*len > LBS_CMD_BUFFER_SIZE)) {
> -		lbs_pr_err("card cmd buffer has invalid # of bytes (%d)\n", *len);
> +		pr_err("card cmd buffer has invalid # of bytes (%d)\n", *len);
>  		goto out;
>  	}
>  
> @@ -472,7 +474,7 @@ static struct sk_buff *if_cs_receive_data(struct lbs_private *priv)
>  
>  	len = if_cs_read16(priv->card, IF_CS_READ_LEN);
>  	if (len == 0 || len > MRVDRV_ETH_RX_PACKET_BUFFER_SIZE) {
> -		lbs_pr_err("card data buffer has invalid # of bytes (%d)\n", len);
> +		pr_err("card data buffer has invalid # of bytes (%d)\n", len);
>  		priv->dev->stats.rx_dropped++;
>  		goto dat_err;
>  	}
> @@ -644,8 +646,8 @@ static int if_cs_prog_helper(struct if_cs_card *card, const struct firmware *fw)
>  		ret = if_cs_poll_while_fw_download(card, IF_CS_CARD_STATUS,
>  			IF_CS_BIT_COMMAND);
>  		if (ret < 0) {
> -			lbs_pr_err("can't download helper at 0x%x, ret %d\n",
> -				sent, ret);
> +			pr_err("can't download helper at 0x%x, ret %d\n",
> +			       sent, ret);
>  			goto done;
>  		}
>  
> @@ -675,7 +677,7 @@ static int if_cs_prog_real(struct if_cs_card *card, const struct firmware *fw)
>  	ret = if_cs_poll_while_fw_download(card, IF_CS_SQ_READ_LOW,
>  		IF_CS_SQ_HELPER_OK);
>  	if (ret < 0) {
> -		lbs_pr_err("helper firmware doesn't answer\n");
> +		pr_err("helper firmware doesn't answer\n");
>  		goto done;
>  	}
>  
> @@ -683,13 +685,13 @@ static int if_cs_prog_real(struct if_cs_card *card, const struct firmware *fw)
>  		len = if_cs_read16(card, IF_CS_SQ_READ_LOW);
>  		if (len & 1) {
>  			retry++;
> -			lbs_pr_info("odd, need to retry this firmware block\n");
> +			pr_info("odd, need to retry this firmware block\n");
>  		} else {
>  			retry = 0;
>  		}
>  
>  		if (retry > 20) {
> -			lbs_pr_err("could not download firmware\n");
> +			pr_err("could not download firmware\n");
>  			ret = -ENODEV;
>  			goto done;
>  		}
> @@ -709,14 +711,14 @@ static int if_cs_prog_real(struct if_cs_card *card, const struct firmware *fw)
>  		ret = if_cs_poll_while_fw_download(card, IF_CS_CARD_STATUS,
>  			IF_CS_BIT_COMMAND);
>  		if (ret < 0) {
> -			lbs_pr_err("can't download firmware at 0x%x\n", sent);
> +			pr_err("can't download firmware at 0x%x\n", sent);
>  			goto done;
>  		}
>  	}
>  
>  	ret = if_cs_poll_while_fw_download(card, IF_CS_SCRATCH, 0x5a);
>  	if (ret < 0)
> -		lbs_pr_err("firmware download failed\n");
> +		pr_err("firmware download failed\n");
>  
>  done:
>  	lbs_deb_leave_args(LBS_DEB_CS, "ret %d", ret);
> @@ -750,7 +752,7 @@ static int if_cs_host_to_card(struct lbs_private *priv,
>  		ret = if_cs_send_cmd(priv, buf, nb);
>  		break;
>  	default:
> -		lbs_pr_err("%s: unsupported type %d\n", __func__, type);
> +		pr_err("%s: unsupported type %d\n", __func__, type);
>  	}
>  
>  	lbs_deb_leave_args(LBS_DEB_CS, "ret %d", ret);
> @@ -779,7 +781,7 @@ static int if_cs_ioprobe(struct pcmcia_device *p_dev, void *priv_data)
>  	p_dev->resource[0]->flags |= IO_DATA_PATH_WIDTH_AUTO;
>  
>  	if (p_dev->resource[1]->end) {
> -		lbs_pr_err("wrong CIS (check number of IO windows)\n");
> +		pr_err("wrong CIS (check number of IO windows)\n");
>  		return -ENODEV;
>  	}
>  
> @@ -800,7 +802,7 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>  
>  	card = kzalloc(sizeof(struct if_cs_card), GFP_KERNEL);
>  	if (!card) {
> -		lbs_pr_err("error in kzalloc\n");
> +		pr_err("error in kzalloc\n");
>  		goto out;
>  	}
>  	card->p_dev = p_dev;
> @@ -809,7 +811,7 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>  	p_dev->config_flags |= CONF_ENABLE_IRQ | CONF_AUTO_SET_IO;
>  
>  	if (pcmcia_loop_config(p_dev, if_cs_ioprobe, NULL)) {
> -		lbs_pr_err("error in pcmcia_loop_config\n");
> +		pr_err("error in pcmcia_loop_config\n");
>  		goto out1;
>  	}
>  
> @@ -825,14 +827,14 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>  	card->iobase = ioport_map(p_dev->resource[0]->start,
>  				resource_size(p_dev->resource[0]));
>  	if (!card->iobase) {
> -		lbs_pr_err("error in ioport_map\n");
> +		pr_err("error in ioport_map\n");
>  		ret = -EIO;
>  		goto out1;
>  	}
>  
>  	ret = pcmcia_enable_device(p_dev);
>  	if (ret) {
> -		lbs_pr_err("error in pcmcia_enable_device\n");
> +		pr_err("error in pcmcia_enable_device\n");
>  		goto out2;
>  	}
>  
> @@ -847,8 +849,8 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>  
>  	card->model = get_model(p_dev->manf_id, p_dev->card_id);
>  	if (card->model == MODEL_UNKNOWN) {
> -		lbs_pr_err("unsupported manf_id 0x%04x / card_id 0x%04x\n",
> -			   p_dev->manf_id, p_dev->card_id);
> +		pr_err("unsupported manf_id 0x%04x / card_id 0x%04x\n",
> +		       p_dev->manf_id, p_dev->card_id);
>  		goto out2;
>  	}
>  
> @@ -857,20 +859,20 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>  	if (card->model == MODEL_8305) {
>  		card->align_regs = 1;
>  		if (prod_id < IF_CS_CF8305_B1_REV) {
> -			lbs_pr_err("8305 rev B0 and older are not supported\n");
> +			pr_err("8305 rev B0 and older are not supported\n");
>  			ret = -ENODEV;
>  			goto out2;
>  		}
>  	}
>  
>  	if ((card->model == MODEL_8381) && prod_id < IF_CS_CF8381_B3_REV) {
> -		lbs_pr_err("8381 rev B2 and older are not supported\n");
> +		pr_err("8381 rev B2 and older are not supported\n");
>  		ret = -ENODEV;
>  		goto out2;
>  	}
>  
>  	if ((card->model == MODEL_8385) && prod_id < IF_CS_CF8385_B1_REV) {
> -		lbs_pr_err("8385 rev B0 and older are not supported\n");
> +		pr_err("8385 rev B0 and older are not supported\n");
>  		ret = -ENODEV;
>  		goto out2;
>  	}
> @@ -878,7 +880,7 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>  	ret = lbs_get_firmware(&p_dev->dev, NULL, NULL, card->model,
>  				&fw_table[0], &helper, &mainfw);
>  	if (ret) {
> -		lbs_pr_err("failed to find firmware (%d)\n", ret);
> +		pr_err("failed to find firmware (%d)\n", ret);
>  		goto out2;
>  	}
>  
> @@ -909,7 +911,7 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>  	ret = request_irq(p_dev->irq, if_cs_interrupt,
>  		IRQF_SHARED, DRV_NAME, card);
>  	if (ret) {
> -		lbs_pr_err("error in request_irq\n");
> +		pr_err("error in request_irq\n");
>  		goto out3;
>  	}
>  
> @@ -920,7 +922,7 @@ static int if_cs_probe(struct pcmcia_device *p_dev)
>  
>  	/* And finally bring the card up */
>  	if (lbs_start_card(priv) != 0) {
> -		lbs_pr_err("could not activate card\n");
> +		pr_err("could not activate card\n");
>  		goto out3;
>  	}
>  
> diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
> index b4de0ca..ab86779 100644
> --- a/drivers/net/wireless/libertas/if_sdio.c
> +++ b/drivers/net/wireless/libertas/if_sdio.c
> @@ -26,6 +26,8 @@
>   * if_sdio_card_to_host() to pad the data.
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/kernel.h>
>  #include <linux/moduleparam.h>
>  #include <linux/slab.h>
> @@ -409,7 +411,7 @@ static int if_sdio_card_to_host(struct if_sdio_card *card)
>  
>  out:
>  	if (ret)
> -		lbs_pr_err("problem fetching packet from firmware\n");
> +		pr_err("problem fetching packet from firmware\n");
>  
>  	lbs_deb_leave_args(LBS_DEB_SDIO, "ret %d", ret);
>  
> @@ -446,7 +448,7 @@ static void if_sdio_host_to_card_worker(struct work_struct *work)
>  		}
>  
>  		if (ret)
> -			lbs_pr_err("error %d sending packet to firmware\n", ret);
> +			pr_err("error %d sending packet to firmware\n", ret);
>  
>  		sdio_release_host(card->func);
>  
> @@ -555,7 +557,7 @@ release:
>  
>  out:
>  	if (ret)
> -		lbs_pr_err("failed to load helper firmware\n");
> +		pr_err("failed to load helper firmware\n");
>  
>  	lbs_deb_leave_args(LBS_DEB_SDIO, "ret %d", ret);
>  	return ret;
> @@ -669,7 +671,7 @@ release:
>  
>  out:
>  	if (ret)
> -		lbs_pr_err("failed to load firmware\n");
> +		pr_err("failed to load firmware\n");
>  
>  	lbs_deb_leave_args(LBS_DEB_SDIO, "ret %d", ret);
>  	return ret;
> @@ -723,7 +725,7 @@ static int if_sdio_prog_firmware(struct if_sdio_card *card)
>  	ret = lbs_get_firmware(&card->func->dev, lbs_helper_name, lbs_fw_name,
>  				card->model, &fw_table[0], &helper, &mainfw);
>  	if (ret) {
> -		lbs_pr_err("failed to find firmware (%d)\n", ret);
> +		pr_err("failed to find firmware (%d)\n", ret);
>  		goto out;
>  	}
>  
> @@ -849,7 +851,7 @@ static int if_sdio_enter_deep_sleep(struct lbs_private *priv)
>  	ret = __lbs_cmd(priv, CMD_802_11_DEEP_SLEEP, &cmd, sizeof(cmd),
>  			lbs_cmd_copyback, (unsigned long) &cmd);
>  	if (ret)
> -		lbs_pr_err("DEEP_SLEEP cmd failed\n");
> +		pr_err("DEEP_SLEEP cmd failed\n");
>  
>  	mdelay(200);
>  	return ret;
> @@ -865,7 +867,7 @@ static int if_sdio_exit_deep_sleep(struct lbs_private *priv)
>  
>  	sdio_writeb(card->func, HOST_POWER_UP, CONFIGURATION_REG, &ret);
>  	if (ret)
> -		lbs_pr_err("sdio_writeb failed!\n");
> +		pr_err("sdio_writeb failed!\n");
>  
>  	sdio_release_host(card->func);
>  	lbs_deb_leave_args(LBS_DEB_SDIO, "ret %d", ret);
> @@ -882,7 +884,7 @@ static int if_sdio_reset_deep_sleep_wakeup(struct lbs_private *priv)
>  
>  	sdio_writeb(card->func, 0, CONFIGURATION_REG, &ret);
>  	if (ret)
> -		lbs_pr_err("sdio_writeb failed!\n");
> +		pr_err("sdio_writeb failed!\n");
>  
>  	sdio_release_host(card->func);
>  	lbs_deb_leave_args(LBS_DEB_SDIO, "ret %d", ret);
> @@ -961,7 +963,7 @@ static int if_sdio_probe(struct sdio_func *func,
>  	}
>  
>  	if (i == func->card->num_info) {
> -		lbs_pr_err("unable to identify card model\n");
> +		pr_err("unable to identify card model\n");
>  		return -ENODEV;
>  	}
>  
> @@ -995,7 +997,7 @@ static int if_sdio_probe(struct sdio_func *func,
>  			break;
>  	}
>  	if (i == ARRAY_SIZE(fw_table)) {
> -		lbs_pr_err("unknown card model 0x%x\n", card->model);
> +		pr_err("unknown card model 0x%x\n", card->model);
>  		ret = -ENODEV;
>  		goto free;
>  	}
> @@ -1101,7 +1103,7 @@ static int if_sdio_probe(struct sdio_func *func,
>  		lbs_deb_sdio("send function INIT command\n");
>  		if (__lbs_cmd(priv, CMD_FUNC_INIT, &cmd, sizeof(cmd),
>  				lbs_cmd_copyback, (unsigned long) &cmd))
> -			lbs_pr_alert("CMD_FUNC_INIT cmd failed\n");
> +			pr_alert("CMD_FUNC_INIT cmd failed\n");
>  	}
>  
>  	ret = lbs_start_card(priv);
> @@ -1163,7 +1165,7 @@ static void if_sdio_remove(struct sdio_func *func)
>  		if (__lbs_cmd(card->priv, CMD_FUNC_SHUTDOWN,
>  				&cmd, sizeof(cmd), lbs_cmd_copyback,
>  				(unsigned long) &cmd))
> -			lbs_pr_alert("CMD_FUNC_SHUTDOWN cmd failed\n");
> +			pr_alert("CMD_FUNC_SHUTDOWN cmd failed\n");
>  	}
>  
> 
> @@ -1202,21 +1204,19 @@ static int if_sdio_suspend(struct device *dev)
>  
>  	mmc_pm_flag_t flags = sdio_get_host_pm_caps(func);
>  
> -	lbs_pr_info("%s: suspend: PM flags = 0x%x\n",
> -						sdio_func_id(func), flags);
> +	pr_info("%s: suspend: PM flags = 0x%x\n", sdio_func_id(func), flags);
>  
>  	/* If we aren't being asked to wake on anything, we should bail out
>  	 * and let the SD stack power down the card.
>  	 */
>  	if (card->priv->wol_criteria == EHS_REMOVE_WAKEUP) {
> -		lbs_pr_info("Suspend without wake params -- "
> -						"powering down card.");
> +		pr_info("Suspend without wake params -- powering down card\n");
>  		return -ENOSYS;
>  	}
>  
>  	if (!(flags & MMC_PM_KEEP_POWER)) {
> -		lbs_pr_err("%s: cannot remain alive while host is suspended\n",
> -			sdio_func_id(func));
> +		pr_err("%s: cannot remain alive while host is suspended\n",
> +		       sdio_func_id(func));
>  		return -ENOSYS;
>  	}
>  
> @@ -1237,7 +1237,7 @@ static int if_sdio_resume(struct device *dev)
>  	struct if_sdio_card *card = sdio_get_drvdata(func);
>  	int ret;
>  
> -	lbs_pr_info("%s: resume: we're back\n", sdio_func_id(func));
> +	pr_info("%s: resume: we're back\n", sdio_func_id(func));
>  
>  	ret = lbs_resume(card->priv);
>  
> diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
> index 078ef43..bebd9f0 100644
> --- a/drivers/net/wireless/libertas/if_spi.c
> +++ b/drivers/net/wireless/libertas/if_spi.c
> @@ -17,6 +17,8 @@
>   * (at your option) any later version.
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/moduleparam.h>
>  #include <linux/firmware.h>
>  #include <linux/jiffies.h>
> @@ -297,8 +299,7 @@ static int spu_wait_for_u16(struct if_spi_card *card, u16 reg,
>  		}
>  		udelay(100);
>  		if (time_after(jiffies, timeout)) {
> -			lbs_pr_err("%s: timeout with val=%02x, "
> -			       "target_mask=%02x, target=%02x\n",
> +			pr_err("%s: timeout with val=%02x, target_mask=%02x, target=%02x\n",
>  			       __func__, val, target_mask, target);
>  			return -ETIMEDOUT;
>  		}
> @@ -391,7 +392,7 @@ static int spu_set_bus_mode(struct if_spi_card *card, u16 mode)
>  	if (err)
>  		return err;
>  	if ((rval & 0xF) != mode) {
> -		lbs_pr_err("Can't read bus mode register.\n");
> +		pr_err("Can't read bus mode register\n");
>  		return -EIO;
>  	}
>  	return 0;
> @@ -512,7 +513,7 @@ static int if_spi_prog_helper_firmware(struct if_spi_card *card,
>  
>  out:
>  	if (err)
> -		lbs_pr_err("failed to load helper firmware (err=%d)\n", err);
> +		pr_err("failed to load helper firmware (err=%d)\n", err);
>  	lbs_deb_leave_args(LBS_DEB_SPI, "err %d", err);
>  	return err;
>  }
> @@ -531,7 +532,7 @@ static int if_spi_prog_main_firmware_check_len(struct if_spi_card *card,
>  				IF_SPI_HIST_CMD_DOWNLOAD_RDY,
>  				IF_SPI_HIST_CMD_DOWNLOAD_RDY);
>  	if (err) {
> -		lbs_pr_err("timed out waiting for host_int_status\n");
> +		pr_err("timed out waiting for host_int_status\n");
>  		return err;
>  	}
>  
> @@ -541,9 +542,8 @@ static int if_spi_prog_main_firmware_check_len(struct if_spi_card *card,
>  		return err;
>  
>  	if (len > IF_SPI_CMD_BUF_SIZE) {
> -		lbs_pr_err("firmware load device requested a larger "
> -			   "tranfer than we are prepared to "
> -			   "handle. (len = %d)\n", len);
> +		pr_err("firmware load device requested a larger transfer than we are prepared to handle (len = %d)\n",
> +		       len);
>  		return -EIO;
>  	}
>  	if (len & 0x1) {
> @@ -572,8 +572,8 @@ static int if_spi_prog_main_firmware(struct if_spi_card *card,
>  
>  	err = spu_wait_for_u16(card, IF_SPI_SCRATCH_1_REG, 0, 0);
>  	if (err) {
> -		lbs_pr_err("%s: timed out waiting for initial "
> -			   "scratch reg = 0\n", __func__);
> +		pr_err("%s: timed out waiting for initial scratch reg = 0\n",
> +		       __func__);
>  		goto out;
>  	}
>  
> @@ -589,15 +589,13 @@ static int if_spi_prog_main_firmware(struct if_spi_card *card,
>  		if (bytes < 0) {
>  			/* If there are no more bytes left, we would normally
>  			 * expect to have terminated with len = 0 */
> -			lbs_pr_err("Firmware load wants more bytes "
> -				   "than we have to offer.\n");
> +			pr_err("Firmware load wants more bytes than we have to offer\n");
>  			break;
>  		}
>  		if (crc_err) {
>  			/* Previous transfer failed. */
>  			if (++num_crc_errs > MAX_MAIN_FW_LOAD_CRC_ERR) {
> -				lbs_pr_err("Too many CRC errors encountered "
> -					   "in firmware load.\n");
> +				pr_err("Too many CRC errors encountered in firmware load\n");
>  				err = -EIO;
>  				goto out;
>  			}
> @@ -626,21 +624,20 @@ static int if_spi_prog_main_firmware(struct if_spi_card *card,
>  		prev_len = len;
>  	}
>  	if (bytes > prev_len) {
> -		lbs_pr_err("firmware load wants fewer bytes than "
> -			   "we have to offer.\n");
> +		pr_err("firmware load wants fewer bytes than we have to offer\n");
>  	}
>  
>  	/* Confirm firmware download */
>  	err = spu_wait_for_u32(card, IF_SPI_SCRATCH_4_REG,
>  					SUCCESSFUL_FW_DOWNLOAD_MAGIC);
>  	if (err) {
> -		lbs_pr_err("failed to confirm the firmware download\n");
> +		pr_err("failed to confirm the firmware download\n");
>  		goto out;
>  	}
>  
>  out:
>  	if (err)
> -		lbs_pr_err("failed to load firmware (err=%d)\n", err);
> +		pr_err("failed to load firmware (err=%d)\n", err);
>  	lbs_deb_leave_args(LBS_DEB_SPI, "err %d", err);
>  	return err;
>  }
> @@ -677,14 +674,12 @@ static int if_spi_c2h_cmd(struct if_spi_card *card)
>  	if (err)
>  		goto out;
>  	if (!len) {
> -		lbs_pr_err("%s: error: card has no data for host\n",
> -			   __func__);
> +		pr_err("%s: error: card has no data for host\n", __func__);
>  		err = -EINVAL;
>  		goto out;
>  	} else if (len > IF_SPI_CMD_BUF_SIZE) {
> -		lbs_pr_err("%s: error: response packet too large: "
> -			   "%d bytes, but maximum is %d\n",
> -			   __func__, len, IF_SPI_CMD_BUF_SIZE);
> +		pr_err("%s: error: response packet too large: %d bytes, but maximum is %d\n",
> +		       __func__, len, IF_SPI_CMD_BUF_SIZE);
>  		err = -EINVAL;
>  		goto out;
>  	}
> @@ -705,7 +700,7 @@ static int if_spi_c2h_cmd(struct if_spi_card *card)
>  
>  out:
>  	if (err)
> -		lbs_pr_err("%s: err=%d\n", __func__, err);
> +		pr_err("%s: err=%d\n", __func__, err);
>  	lbs_deb_leave(LBS_DEB_SPI);
>  	return err;
>  }
> @@ -725,14 +720,12 @@ static int if_spi_c2h_data(struct if_spi_card *card)
>  	if (err)
>  		goto out;
>  	if (!len) {
> -		lbs_pr_err("%s: error: card has no data for host\n",
> -			   __func__);
> +		pr_err("%s: error: card has no data for host\n", __func__);
>  		err = -EINVAL;
>  		goto out;
>  	} else if (len > MRVDRV_ETH_RX_PACKET_BUFFER_SIZE) {
> -		lbs_pr_err("%s: error: card has %d bytes of data, but "
> -			   "our maximum skb size is %zu\n",
> -			   __func__, len, MRVDRV_ETH_RX_PACKET_BUFFER_SIZE);
> +		pr_err("%s: error: card has %d bytes of data, but our maximum skb size is %zu\n",
> +		       __func__, len, MRVDRV_ETH_RX_PACKET_BUFFER_SIZE);
>  		err = -EINVAL;
>  		goto out;
>  	}
> @@ -763,7 +756,7 @@ free_skb:
>  	dev_kfree_skb(skb);
>  out:
>  	if (err)
> -		lbs_pr_err("%s: err=%d\n", __func__, err);
> +		pr_err("%s: err=%d\n", __func__, err);
>  	lbs_deb_leave(LBS_DEB_SPI);
>  	return err;
>  }
> @@ -785,7 +778,7 @@ static void if_spi_h2c(struct if_spi_card *card,
>  		port_reg = IF_SPI_CMD_RDWRPORT_REG;
>  		break;
>  	default:
> -		lbs_pr_err("can't transfer buffer of type %d\n", type);
> +		pr_err("can't transfer buffer of type %d\n", type);
>  		err = -EINVAL;
>  		goto out;
>  	}
> @@ -799,7 +792,7 @@ out:
>  	kfree(packet);
>  
>  	if (err)
> -		lbs_pr_err("%s: error %d\n", __func__, err);
> +		pr_err("%s: error %d\n", __func__, err);
>  }
>  
>  /* Inform the host about a card event */
> @@ -823,7 +816,7 @@ static void if_spi_e2h(struct if_spi_card *card)
>  	lbs_queue_event(priv, cause & 0xff);
>  out:
>  	if (err)
> -		lbs_pr_err("%s: error %d\n", __func__, err);
> +		pr_err("%s: error %d\n", __func__, err);
>  }
>  
>  static void if_spi_host_to_card_worker(struct work_struct *work)
> @@ -843,7 +836,7 @@ static void if_spi_host_to_card_worker(struct work_struct *work)
>  	err = spu_read_u16(card, IF_SPI_HOST_INT_STATUS_REG,
>  				&hiStatus);
>  	if (err) {
> -		lbs_pr_err("I/O error\n");
> +		pr_err("I/O error\n");
>  		goto err;
>  	}
>  
> @@ -905,7 +898,7 @@ static void if_spi_host_to_card_worker(struct work_struct *work)
>  
>  err:
>  	if (err)
> -		lbs_pr_err("%s: got error %d\n", __func__, err);
> +		pr_err("%s: got error %d\n", __func__, err);
>  
>  	lbs_deb_leave(LBS_DEB_SPI);
>  }
> @@ -927,7 +920,7 @@ static int if_spi_host_to_card(struct lbs_private *priv,
>  	lbs_deb_enter_args(LBS_DEB_SPI, "type %d, bytes %d", type, nb);
>  
>  	if (nb == 0) {
> -		lbs_pr_err("%s: invalid size requested: %d\n", __func__, nb);
> +		pr_err("%s: invalid size requested: %d\n", __func__, nb);
>  		err = -EINVAL;
>  		goto out;
>  	}
> @@ -955,7 +948,7 @@ static int if_spi_host_to_card(struct lbs_private *priv,
>  		spin_unlock_irqrestore(&card->buffer_lock, flags);
>  		break;
>  	default:
> -		lbs_pr_err("can't transfer buffer of type %d", type);
> +		pr_err("can't transfer buffer of type %d\n", type);
>  		err = -EINVAL;
>  		break;
>  	}
> @@ -1016,8 +1009,7 @@ static int if_spi_init_card(struct if_spi_card *card)
>  				break;
>  		}
>  		if (i == ARRAY_SIZE(fw_table)) {
> -			lbs_pr_err("Unsupported chip_id: 0x%02x\n",
> -					card->card_id);
> +			pr_err("Unsupported chip_id: 0x%02x\n", card->card_id);
>  			err = -ENODEV;
>  			goto out;
>  		}
> @@ -1026,7 +1018,7 @@ static int if_spi_init_card(struct if_spi_card *card)
>  					card->card_id, &fw_table[0], &helper,
>  					&mainfw);
>  		if (err) {
> -			lbs_pr_err("failed to find firmware (%d)\n", err);
> +			pr_err("failed to find firmware (%d)\n", err);
>  			goto out;
>  		}
>  
> @@ -1149,7 +1141,7 @@ static int __devinit if_spi_probe(struct spi_device *spi)
>  	err = request_irq(spi->irq, if_spi_host_interrupt,
>  			IRQF_TRIGGER_FALLING, "libertas_spi", card);
>  	if (err) {
> -		lbs_pr_err("can't get host irq line-- request_irq failed\n");
> +		pr_err("can't get host irq line-- request_irq failed\n");
>  		goto terminate_workqueue;
>  	}
>  
> diff --git a/drivers/net/wireless/libertas/if_usb.c b/drivers/net/wireless/libertas/if_usb.c
> index 6524c70..7260791 100644
> --- a/drivers/net/wireless/libertas/if_usb.c
> +++ b/drivers/net/wireless/libertas/if_usb.c
> @@ -1,6 +1,9 @@
>  /**
>    * This file contains functions used in USB interface module.
>    */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/delay.h>
>  #include <linux/moduleparam.h>
>  #include <linux/firmware.h>
> @@ -145,7 +148,7 @@ static void if_usb_write_bulk_callback(struct urb *urb)
>  			lbs_host_to_card_done(priv);
>  	} else {
>  		/* print the failure status number for debug */
> -		lbs_pr_info("URB in failure status: %d\n", urb->status);
> +		pr_info("URB in failure status: %d\n", urb->status);
>  	}
>  }
>  
> @@ -195,7 +198,7 @@ static void if_usb_setup_firmware(struct lbs_private *priv)
>  	wake_method.hdr.size = cpu_to_le16(sizeof(wake_method));
>  	wake_method.action = cpu_to_le16(CMD_ACT_GET);
>  	if (lbs_cmd_with_response(priv, CMD_802_11_FW_WAKE_METHOD, &wake_method)) {
> -		lbs_pr_info("Firmware does not seem to support PS mode\n");
> +		pr_info("Firmware does not seem to support PS mode\n");
>  		priv->fwcapinfo &= ~FW_CAPINFO_PS;
>  	} else {
>  		if (le16_to_cpu(wake_method.method) == CMD_WAKE_METHOD_COMMAND_INT) {
> @@ -204,7 +207,7 @@ static void if_usb_setup_firmware(struct lbs_private *priv)
>  			/* The versions which boot up this way don't seem to
>  			   work even if we set it to the command interrupt */
>  			priv->fwcapinfo &= ~FW_CAPINFO_PS;
> -			lbs_pr_info("Firmware doesn't wake via command interrupt; disabling PS mode\n");
> +			pr_info("Firmware doesn't wake via command interrupt; disabling PS mode\n");
>  		}
>  	}
>  }
> @@ -216,7 +219,7 @@ static void if_usb_fw_timeo(unsigned long priv)
>  	if (cardp->fwdnldover) {
>  		lbs_deb_usb("Download complete, no event. Assuming success\n");
>  	} else {
> -		lbs_pr_err("Download timed out\n");
> +		pr_err("Download timed out\n");
>  		cardp->surprise_removed = 1;
>  	}
>  	wake_up(&cardp->fw_wq);
> @@ -250,7 +253,7 @@ static int if_usb_probe(struct usb_interface *intf,
>  
>  	cardp = kzalloc(sizeof(struct if_usb_card), GFP_KERNEL);
>  	if (!cardp) {
> -		lbs_pr_err("Out of memory allocating private data.\n");
> +		pr_err("Out of memory allocating private data\n");
>  		goto error;
>  	}
>  
> @@ -340,10 +343,10 @@ static int if_usb_probe(struct usb_interface *intf,
>  	usb_set_intfdata(intf, cardp);
>  
>  	if (device_create_file(&priv->dev->dev, &dev_attr_lbs_flash_fw))
> -		lbs_pr_err("cannot register lbs_flash_fw attribute\n");
> +		pr_err("cannot register lbs_flash_fw attribute\n");
>  
>  	if (device_create_file(&priv->dev->dev, &dev_attr_lbs_flash_boot2))
> -		lbs_pr_err("cannot register lbs_flash_boot2 attribute\n");
> +		pr_err("cannot register lbs_flash_boot2 attribute\n");
>  
>  	/*
>  	 * EHS_REMOVE_WAKEUP is not supported on all versions of the firmware.
> @@ -528,7 +531,7 @@ static int __if_usb_submit_rx_urb(struct if_usb_card *cardp,
>  	int ret = -1;
>  
>  	if (!(skb = dev_alloc_skb(MRVDRV_ETH_RX_PACKET_BUFFER_SIZE))) {
> -		lbs_pr_err("No free skb\n");
> +		pr_err("No free skb\n");
>  		goto rx_ret;
>  	}
>  
> @@ -587,7 +590,7 @@ static void if_usb_receive_fwload(struct urb *urb)
>  
>  		if (tmp[0] == cpu_to_le32(CMD_TYPE_INDICATION) &&
>  		    tmp[1] == cpu_to_le32(MACREG_INT_CODE_FIRMWARE_READY)) {
> -			lbs_pr_info("Firmware ready event received\n");
> +			pr_info("Firmware ready event received\n");
>  			wake_up(&cardp->fw_wq);
>  		} else {
>  			lbs_deb_usb("Waiting for confirmation; got %x %x\n",
> @@ -614,20 +617,20 @@ static void if_usb_receive_fwload(struct urb *urb)
>  			    bootcmdresp.magic == cpu_to_le32(CMD_TYPE_DATA) ||
>  			    bootcmdresp.magic == cpu_to_le32(CMD_TYPE_INDICATION)) {
>  				if (!cardp->bootcmdresp)
> -					lbs_pr_info("Firmware already seems alive; resetting\n");
> +					pr_info("Firmware already seems alive; resetting\n");
>  				cardp->bootcmdresp = -1;
>  			} else {
> -				lbs_pr_info("boot cmd response wrong magic number (0x%x)\n",
> +				pr_info("boot cmd response wrong magic number (0x%x)\n",
>  					    le32_to_cpu(bootcmdresp.magic));
>  			}
>  		} else if ((bootcmdresp.cmd != BOOT_CMD_FW_BY_USB) &&
>  			   (bootcmdresp.cmd != BOOT_CMD_UPDATE_FW) &&
>  			   (bootcmdresp.cmd != BOOT_CMD_UPDATE_BOOT2)) {
> -			lbs_pr_info("boot cmd response cmd_tag error (%d)\n",
> -				    bootcmdresp.cmd);
> +			pr_info("boot cmd response cmd_tag error (%d)\n",
> +				bootcmdresp.cmd);
>  		} else if (bootcmdresp.result != BOOT_CMD_RESP_OK) {
> -			lbs_pr_info("boot cmd response result error (%d)\n",
> -				    bootcmdresp.result);
> +			pr_info("boot cmd response result error (%d)\n",
> +				bootcmdresp.result);
>  		} else {
>  			cardp->bootcmdresp = 1;
>  			lbs_deb_usbd(&cardp->udev->dev,
> @@ -892,7 +895,7 @@ static int check_fwfile_format(const uint8_t *data, uint32_t totlen)
>  	} while (!exit);
>  
>  	if (ret)
> -		lbs_pr_err("firmware file format check FAIL\n");
> +		pr_err("firmware file format check FAIL\n");
>  	else
>  		lbs_deb_fw("firmware file format check PASS\n");
>  
> @@ -989,7 +992,7 @@ static int __if_usb_prog_firmware(struct if_usb_card *cardp,
>  
>  	ret = get_fw(cardp, fwname);
>  	if (ret) {
> -		lbs_pr_err("failed to find firmware (%d)\n", ret);
> +		pr_err("failed to find firmware (%d)\n", ret);
>  		goto done;
>  	}
>  
> @@ -1064,13 +1067,13 @@ restart:
>  	usb_kill_urb(cardp->rx_urb);
>  
>  	if (!cardp->fwdnldover) {
> -		lbs_pr_info("failed to load fw, resetting device!\n");
> +		pr_info("failed to load fw, resetting device!\n");
>  		if (--reset_count >= 0) {
>  			if_usb_reset_device(cardp);
>  			goto restart;
>  		}
>  
> -		lbs_pr_info("FW download failure, time = %d ms\n", i * 100);
> +		pr_info("FW download failure, time = %d ms\n", i * 100);
>  		ret = -EIO;
>  		goto release_fw;
>  	}
> diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
> index ca8149c..639461b 100644
> --- a/drivers/net/wireless/libertas/main.c
> +++ b/drivers/net/wireless/libertas/main.c
> @@ -4,6 +4,8 @@
>    * thread etc..
>    */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/moduleparam.h>
>  #include <linux/delay.h>
>  #include <linux/etherdevice.h>
> @@ -153,7 +155,7 @@ static void lbs_tx_timeout(struct net_device *dev)
>  
>  	lbs_deb_enter(LBS_DEB_TX);
>  
> -	lbs_pr_err("tx watch dog timeout\n");
> +	pr_err("tx watch dog timeout\n");
>  
>  	dev->trans_start = jiffies; /* prevent tx timeout */
>  
> @@ -462,7 +464,7 @@ static int lbs_thread(void *data)
>  		if (priv->cmd_timed_out && priv->cur_cmd) {
>  			struct cmd_ctrl_node *cmdnode = priv->cur_cmd;
>  
> -			lbs_pr_info("Timeout submitting command 0x%04x\n",
> +			pr_info("Timeout submitting command 0x%04x\n",
>  				le16_to_cpu(cmdnode->cmdbuf->command));
>  			lbs_complete_command(priv, cmdnode, -ETIMEDOUT);
>  			if (priv->reset_card)
> @@ -490,8 +492,7 @@ static int lbs_thread(void *data)
>  				 * after firmware fixes it
>  				 */
>  				priv->psstate = PS_STATE_AWAKE;
> -				lbs_pr_alert("ignore PS_SleepConfirm in "
> -					"non-connected state\n");
> +				pr_alert("ignore PS_SleepConfirm in non-connected state\n");
>  			}
>  		}
>  
> @@ -585,7 +586,7 @@ int lbs_suspend(struct lbs_private *priv)
>  	if (priv->is_deep_sleep) {
>  		ret = lbs_set_deep_sleep(priv, 0);
>  		if (ret) {
> -			lbs_pr_err("deep sleep cancellation failed: %d\n", ret);
> +			pr_err("deep sleep cancellation failed: %d\n", ret);
>  			return ret;
>  		}
>  		priv->deep_sleep_required = 1;
> @@ -618,7 +619,7 @@ int lbs_resume(struct lbs_private *priv)
>  		priv->deep_sleep_required = 0;
>  		ret = lbs_set_deep_sleep(priv, 1);
>  		if (ret)
> -			lbs_pr_err("deep sleep activation failed: %d\n", ret);
> +			pr_err("deep sleep activation failed: %d\n", ret);
>  	}
>  
>  	if (priv->setup_fw_on_resume)
> @@ -644,7 +645,7 @@ static void lbs_cmd_timeout_handler(unsigned long data)
>  	if (!priv->cur_cmd)
>  		goto out;
>  
> -	lbs_pr_info("command 0x%04x timed out\n",
> +	pr_info("command 0x%04x timed out\n",
>  		le16_to_cpu(priv->cur_cmd->cmdbuf->command));
>  
>  	priv->cmd_timed_out = 1;
> @@ -748,7 +749,7 @@ static int lbs_init_adapter(struct lbs_private *priv)
>  
>  	/* Allocate the command buffers */
>  	if (lbs_allocate_cmd_buffer(priv)) {
> -		lbs_pr_err("Out of memory allocating command buffers\n");
> +		pr_err("Out of memory allocating command buffers\n");
>  		ret = -ENOMEM;
>  		goto out;
>  	}
> @@ -758,7 +759,7 @@ static int lbs_init_adapter(struct lbs_private *priv)
>  	/* Create the event FIFO */
>  	ret = kfifo_alloc(&priv->event_fifo, sizeof(u32) * 16, GFP_KERNEL);
>  	if (ret) {
> -		lbs_pr_err("Out of memory allocating event FIFO buffer\n");
> +		pr_err("Out of memory allocating event FIFO buffer\n");
>  		goto out;
>  	}
>  
> @@ -809,7 +810,7 @@ struct lbs_private *lbs_add_card(void *card, struct device *dmdev)
>  	/* Allocate an Ethernet device and register it */
>  	wdev = lbs_cfg_alloc(dmdev);
>  	if (IS_ERR(wdev)) {
> -		lbs_pr_err("cfg80211 init failed\n");
> +		pr_err("cfg80211 init failed\n");
>  		goto done;
>  	}
>  
> @@ -818,7 +819,7 @@ struct lbs_private *lbs_add_card(void *card, struct device *dmdev)
>  	priv->wdev = wdev;
>  
>  	if (lbs_init_adapter(priv)) {
> -		lbs_pr_err("failed to initialize adapter structure.\n");
> +		pr_err("failed to initialize adapter structure\n");
>  		goto err_wdev;
>  	}
>  
> @@ -950,7 +951,7 @@ int lbs_start_card(struct lbs_private *priv)
>  		goto done;
>  
>  	if (lbs_cfg_register(priv)) {
> -		lbs_pr_err("cannot register device\n");
> +		pr_err("cannot register device\n");
>  		goto done;
>  	}
>  
> @@ -960,7 +961,7 @@ int lbs_start_card(struct lbs_private *priv)
>  
>  	lbs_debugfs_init_one(priv, dev);
>  
> -	lbs_pr_info("%s: Marvell WLAN 802.11 adapter\n", dev->name);
> +	pr_info("%s: Marvell WLAN 802.11 adapter\n", dev->name);
>  
>  	ret = 0;
>  
> @@ -1087,16 +1088,14 @@ int lbs_get_firmware(struct device *dev, const char *user_helper,
>  	if (user_helper) {
>  		ret = request_firmware(helper, user_helper, dev);
>  		if (ret) {
> -			lbs_pr_err("couldn't find helper firmware %s",
> -					user_helper);
> +			pr_err("couldn't find helper firmware %s", user_helper);
>  			goto fail;
>  		}
>  	}
>  	if (user_mainfw) {
>  		ret = request_firmware(mainfw, user_mainfw, dev);
>  		if (ret) {
> -			lbs_pr_err("couldn't find main firmware %s",
> -					user_mainfw);
> +			pr_err("couldn't find main firmware %s", user_mainfw);
>  			goto fail;
>  		}
>  	}
> diff --git a/drivers/net/wireless/libertas/mesh.c b/drivers/net/wireless/libertas/mesh.c
> index 9d097b9..f7c51cb 100644
> --- a/drivers/net/wireless/libertas/mesh.c
> +++ b/drivers/net/wireless/libertas/mesh.c
> @@ -1,3 +1,5 @@
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/delay.h>
>  #include <linux/etherdevice.h>
>  #include <linux/netdevice.h>
> @@ -246,7 +248,7 @@ int lbs_init_mesh(struct lbs_private *priv)
>  		lbs_add_mesh(priv);
>  
>  		if (device_create_file(&dev->dev, &dev_attr_lbs_mesh))
> -			lbs_pr_err("cannot register lbs_mesh attribute\n");
> +			pr_err("cannot register lbs_mesh attribute\n");
>  
>  		ret = 1;
>  	}
> @@ -374,7 +376,7 @@ int lbs_add_mesh(struct lbs_private *priv)
>  	/* Register virtual mesh interface */
>  	ret = register_netdev(mesh_dev);
>  	if (ret) {
> -		lbs_pr_err("cannot register mshX virtual interface\n");
> +		pr_err("cannot register mshX virtual interface\n");
>  		goto err_free;
>  	}
>  
> @@ -926,7 +928,7 @@ static ssize_t mesh_id_get(struct device *dev, struct device_attribute *attr,
>  		return ret;
>  
>  	if (defs.meshie.val.mesh_id_len > IEEE80211_MAX_SSID_LEN) {
> -		lbs_pr_err("inconsistent mesh ID length");
> +		pr_err("inconsistent mesh ID length\n");
>  		defs.meshie.val.mesh_id_len = IEEE80211_MAX_SSID_LEN;
>  	}
>  
> diff --git a/drivers/net/wireless/libertas/rx.c b/drivers/net/wireless/libertas/rx.c
> index a2b1df2..fd045aa 100644
> --- a/drivers/net/wireless/libertas/rx.c
> +++ b/drivers/net/wireless/libertas/rx.c
> @@ -1,6 +1,9 @@
>  /**
>    * This file contains the handling of RX in wlan driver.
>    */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/etherdevice.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
> @@ -191,7 +194,7 @@ static u8 convert_mv_rate_to_radiotap(u8 rate)
>  	case 12:		/*  54 Mbps */
>  		return 108;
>  	}
> -	lbs_pr_alert("Invalid Marvell WLAN rate %i\n", rate);
> +	pr_alert("Invalid Marvell WLAN rate %i\n", rate);
>  	return 0;
>  }
>  
> @@ -248,7 +251,7 @@ static int process_rxed_802_11_packet(struct lbs_private *priv,
>  	/* add space for the new radio header */
>  	if ((skb_headroom(skb) < sizeof(struct rx_radiotap_hdr)) &&
>  	    pskb_expand_head(skb, sizeof(struct rx_radiotap_hdr), 0, GFP_ATOMIC)) {
> -		lbs_pr_alert("%s: couldn't pskb_expand_head\n", __func__);
> +		pr_alert("%s: couldn't pskb_expand_head\n", __func__);
>  		ret = -ENOMEM;
>  		kfree_skb(skb);
>  		goto done;

^ permalink raw reply

* Re: [PATCH] net: ehea: fix wrongly-reported supported modes
From: Kleber Sacilotto de Souza @ 2011-05-04 21:59 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev
In-Reply-To: <1304458460.2873.18.camel@bwh-desktop>

On Tue, 2011-05-03 at 22:34 +0100, Ben Hutchings wrote:
> On Tue, 2011-05-03 at 21:16 +0100, Ben Hutchings wrote:
> > On Tue, 2011-05-03 at 16:42 -0300, Kleber Sacilotto de Souza wrote:
> > > Currently EHEA reports to ethtool as supporting 10000baseT_Full and
> > > FIBRE independent of the hardware configuration. However, these
> > > capabilities should be reported only if the physical port and
> > > the medium support them, which is the case where the physical port
> > > is connected at 10Gb.
> > >
> > > Signed-off-by: Kleber Sacilotto de Souza <klebers@linux.vnet.ibm.com>
> > > ---
> > >  drivers/net/ehea/ehea_ethtool.c |   21 ++++++++++++++-------
> > >  1 files changed, 14 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/net/ehea/ehea_ethtool.c b/drivers/net/ehea/ehea_ethtool.c
> > > index 3e2e734..04716c2 100644
> > > --- a/drivers/net/ehea/ehea_ethtool.c
> > > +++ b/drivers/net/ehea/ehea_ethtool.c
> > > @@ -55,15 +55,22 @@ static int ehea_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> > >  		cmd->duplex = -1;
> > >  	}
> > > 
> > > -	cmd->supported = (SUPPORTED_10000baseT_Full | SUPPORTED_1000baseT_Full
> > > -		       | SUPPORTED_100baseT_Full |  SUPPORTED_100baseT_Half
> > > -		       | SUPPORTED_10baseT_Full | SUPPORTED_10baseT_Half
> > > -		       | SUPPORTED_Autoneg | SUPPORTED_FIBRE);
> > > +	cmd->supported = (SUPPORTED_1000baseT_Full | SUPPORTED_100baseT_Full
> > > +		       | SUPPORTED_100baseT_Half | SUPPORTED_10baseT_Full
> > > +		       | SUPPORTED_10baseT_Half | SUPPORTED_Autoneg);
> > > 
> > > -	cmd->advertising = (ADVERTISED_10000baseT_Full | ADVERTISED_Autoneg
> > > -			 | ADVERTISED_FIBRE);
> > > +	cmd->advertising = ADVERTISED_Autoneg;
> > > +
> > > +	if (cmd->speed == SPEED_10000) {
> > > +		cmd->supported |= (SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE);
> > > +		cmd->advertising |= (ADVERTISED_10000baseT_Full | ADVERTISED_FIBRE);
> > > +		cmd->port = PORT_FIBRE;
> > > +	} else {
> > > +		cmd->supported |= SUPPORTED_TP;
> > > +		cmd->advertising |= (ADVERTISED_1000baseT_Full | ADVERTISED_TP);
> > > +		cmd->port = PORT_TP;
> > > +	}
> > 
> > This doesn't make any sense.  If the current speed is 10G, then the
> > driver also claims to support speeds of 10M, 100M, 1G and 10G.  But then
>                                                                ^
>                                                            on fibre
> 
> > if the speed actually is <10G, the driver claims to support TP.  What's
> > going on here?

You are right. This patch was based on very vague hardware specs that
wasn't making much sense. I have more details about the hardware now, I
will send another patch soon.

> > 
> > (Also, claiming to support BASE-T modes on non-TP media is bogus, though
> > I understand why people are doing it.)
> > 
> > Ben.
> > 
> > > -	cmd->port = PORT_FIBRE;
> > >  	cmd->autoneg = port->autoneg == 1 ? AUTONEG_ENABLE : AUTONEG_DISABLE;
> > > 
> > >  	return 0;
> > 
> 

-- 
Kleber S. Souza
IBM Linux Technology Center


^ permalink raw reply

* Re: [PATCH 09/18] virtio: use avail_event index
From: Tom Lendacky @ 2011-05-04 21:58 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, Rusty Russell, Carsten Otte, Christian Borntraeger,
	linux390, Martin Schwidefsky, Heiko Carstens, Shirley Ma, lguest,
	virtualization, netdev, linux-s390, kvm, Krishna Kumar, steved,
	habanero
In-Reply-To: <8bba6a0a8eee17e741c5155b04ff1b1c9f34bf94.1304541919.git.mst@redhat.com>


On Wednesday, May 04, 2011 03:51:47 PM Michael S. Tsirkin wrote:
> Use the new avail_event feature to reduce the number
> of exits from the guest.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/virtio/virtio_ring.c |   39
> ++++++++++++++++++++++++++++++++++++++- 1 files changed, 38 insertions(+),
> 1 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 3a3ed75..262dfe6 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -82,6 +82,15 @@ struct vring_virtqueue
>  	/* Host supports indirect buffers */
>  	bool indirect;
> 
> +	/* Host publishes avail event idx */
> +	bool event;
> +
> +	/* Is kicked_avail below valid? */
> +	bool kicked_avail_valid;
> +
> +	/* avail idx value we already kicked. */
> +	u16 kicked_avail;
> +
>  	/* Number of free buffers */
>  	unsigned int num_free;
>  	/* Head of free buffer list. */
> @@ -228,6 +237,12 @@ add_head:
>  	 * new available array entries. */
>  	virtio_wmb();
>  	vq->vring.avail->idx++;
> +	/* If the driver never bothers to kick in a very long while,
> +	 * avail index might wrap around. If that happens, invalidate
> +	 * kicked_avail index we stored. TODO: make sure all drivers
> +	 * kick at least once in 2^16 and remove this. */
> +	if (unlikely(vq->vring.avail->idx == vq->kicked_avail))
> +		vq->kicked_avail_valid = true;

vq->kicked_avail_valid should be set to false here.

Tom

> 
>  	pr_debug("Added buffer head %i to %p\n", head, vq);
>  	END_USE(vq);
> @@ -236,6 +251,23 @@ add_head:
>  }
>  EXPORT_SYMBOL_GPL(virtqueue_add_buf_gfp);
> 
> +
> +static bool vring_notify(struct vring_virtqueue *vq)
> +{
> +	u16 old, new;
> +	bool v;
> +	if (!vq->event)
> +		return !(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY);
> +
> +	v = vq->kicked_avail_valid;
> +	old = vq->kicked_avail;
> +	new = vq->kicked_avail = vq->vring.avail->idx;
> +	vq->kicked_avail_valid = true;
> +	if (unlikely(!v))
> +		return true;
> +	return vring_need_event(vring_avail_event(&vq->vring), new, old);
> +}
> +
>  void virtqueue_kick(struct virtqueue *_vq)
>  {
>  	struct vring_virtqueue *vq = to_vvq(_vq);
> @@ -244,7 +276,7 @@ void virtqueue_kick(struct virtqueue *_vq)
>  	/* Need to update avail index before checking if we should notify */
>  	virtio_mb();
> 
> -	if (!(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY))
> +	if (vring_notify(vq))
>  		/* Prod other side to tell it about changes. */
>  		vq->notify(&vq->vq);
> 
> @@ -437,6 +469,8 @@ struct virtqueue *vring_new_virtqueue(unsigned int num,
>  	vq->vq.name = name;
>  	vq->notify = notify;
>  	vq->broken = false;
> +	vq->kicked_avail_valid = false;
> +	vq->kicked_avail = 0;
>  	vq->last_used_idx = 0;
>  	list_add_tail(&vq->vq.list, &vdev->vqs);
>  #ifdef DEBUG
> @@ -444,6 +478,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int num,
>  #endif
> 
>  	vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC);
> +	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_AVAIL_EVENT_IDX);
> 
>  	/* No callback?  Tell other side not to bother us. */
>  	if (!callback)
> @@ -482,6 +517,8 @@ void vring_transport_features(struct virtio_device
> *vdev) break;
>  		case VIRTIO_RING_F_USED_EVENT_IDX:
>  			break;
> +		case VIRTIO_RING_F_AVAIL_EVENT_IDX:
> +			break;
>  		default:
>  			/* We don't understand this bit. */
>  			clear_bit(i, vdev->features);

^ permalink raw reply

* Lockdep splat for rt8169
From: Ben Greear @ 2011-05-04 21:56 UTC (permalink / raw)
  To: netdev; +Cc: Francois Romieu

This is from un-modified 39-rc6, with the slub cmpxcg patch posted today on lkml.

Seems to be the first post 2.6.38 kernel that will boot stable
on this system!

I previously reported the timer issue, but perhaps the lock
debugging will help.

[ INFO: inconsistent lock state ]
2.6.39-rc6+ #22
---------------------------------
inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
udevd/2410 [HC1[1]:SC0[0]:HE0:SE1] takes:
  (/home/greearb/git/linux-2.6/net/core/link_watch.c:35){?.-...}, at: [<c0445bb6>] del_timer_sync+0x0/0xa7
{HARDIRQ-ON-W} state was registered at:
   [<c046293d>] __lock_acquire+0x2b5/0xb77
   [<c046329f>] lock_acquire+0xa0/0xc4
   [<c044548a>] run_timer_softirq+0x142/0x232
   [<c043fbaa>] __do_softirq+0xb1/0x17c
irq event stamp: 138
hardirqs last  enabled at (137): [<c04b01b9>] get_page_from_freelist+0x28c/0x3c9
hardirqs last disabled at (138): [<c07f5927>] common_interrupt+0x27/0x40
softirqs last  enabled at (0): [<c0438c53>] copy_process+0x301/0xf1b
softirqs last disabled at (0): [<  (null)>]   (null)

other info that might help us debug this:
2 locks held by udevd/2410:
  #0:  (&sig->cred_guard_mutex){+.+.+.}, at: [<c04ea205>] prepare_bprm_creds+0x25/0x5a
  #1:  (&(&tp->lock)->rlock){-.-...}, at: [<f88c34ca>] __rtl8169_check_link_status+0x25/0xb4 [r8169]

stack backtrace:
Pid: 2410, comm: udevd Not tainted 2.6.39-rc6+ #22
Call Trace:
  [<c04617dc>] valid_state+0x131/0x144
  [<c04618de>] mark_lock+0xef/0x1de
  [<c0461fa1>] ? print_irq_inversion_bug+0xf0/0xf0
  [<c04628cf>] __lock_acquire+0x247/0xb77
  [<c0445bb6>] ? get_next_timer_interrupt+0x1d2/0x1d2
  [<c046329f>] lock_acquire+0xa0/0xc4
  [<c0445bb6>] ? get_next_timer_interrupt+0x1d2/0x1d2
  [<c0445bef>] del_timer_sync+0x39/0xa7
  [<c0445bb6>] ? get_next_timer_interrupt+0x1d2/0x1d2
  [<c07579ae>] linkwatch_schedule_work+0x6d/0x88
  [<c0757a76>] linkwatch_fire_event+0xad/0xb2
  [<c075ee8c>] netif_carrier_on+0x28/0x39
  [<f88c34f9>] __rtl8169_check_link_status+0x54/0xb4 [r8169]
  [<f88c3e22>] rtl8169_interrupt+0x1f4/0x298 [r8169]
  [<c0483fe0>] handle_irq_event_percpu+0x58/0x17b
  [<c0484134>] handle_irq_event+0x31/0x48
  [<c0485c2f>] ? handle_percpu_irq+0x40/0x40
  [<c0485cbe>] handle_edge_irq+0x8f/0xb1
  <IRQ>  [<c0403afe>] ? do_IRQ+0x3c/0x95
  [<c07f592e>] ? common_interrupt+0x2e/0x40
  [<c04b00d8>] ? get_page_from_freelist+0x1ab/0x3c9
  [<c04b0bd6>] ? __alloc_pages_nodemask+0x60e/0x66f
  [<c05b6061>] ? blk_finish_plug+0x12/0x2d
  [<c04254b1>] ? pte_alloc_one+0x1c/0x37
  [<c04c1fb7>] ? __pte_alloc+0x1d/0xf3
  [<c04c425f>] ? handle_mm_fault+0xee/0x150
  [<c04c4518>] ? __get_user_pages+0x257/0x39b
  [<c04c46d5>] ? get_user_pages+0x39/0x40
  [<c04eb064>] ? get_arg_page+0x35/0x8e
  [<c05cfc9c>] ? strnlen_user+0x20/0x3e
  [<c04eb1a8>] ? copy_strings+0xeb/0x1b3
  [<c04eb291>] ? copy_strings_kernel+0x21/0x30
  [<c04eb674>] ? do_execve+0x11d/0x22d
  [<c040830a>] ? sys_execve+0x31/0x54
  [<c07f5452>] ? ptregs_execve+0x12/0x20
  [<c07f535c>] ? sysenter_do_call+0x12/0x38
------------[ cut here ]------------
WARNING: at /home/greearb/git/linux-2.6/kernel/timer.c:1012 del_timer_sync+0x90/0xa7()
Hardware name: To Be Filled By O.E.M.
Modules linked in: veth 8021q garp stp llc fuse macvlan pktgen coretemp hwmon nfs lockd fscache auth_rpcgss nfs_acl sunrpc ipv6 uinput arc4 ecb 
snd_hda_codec_realtek ath9k snd_hda_intel snd_hda_codec mac80211 snd_hwdep snd_seq ath9k_common snd_seq_device snd_pcm microcode ath9k_hw ath cfg80211 snd_timer 
iTCO_wdt i2c_i801 pcspkr snd serio_raw iTCO_vendor_support r8169 soundcore snd_page_alloc mii i915 drm_kms_helper drm i2c_algo_bit video [last unloaded: 
scsi_wait_scan]
Pid: 2410, comm: udevd Not tainted 2.6.39-rc6+ #22
Call Trace:
  [<c043a0e2>] warn_slowpath_common+0x6a/0x7f
  [<c0445c46>] ? del_timer_sync+0x90/0xa7
  [<c043a10b>] warn_slowpath_null+0x14/0x18
  [<c0445c46>] del_timer_sync+0x90/0xa7
  [<c07579ae>] linkwatch_schedule_work+0x6d/0x88
  [<c0757a76>] linkwatch_fire_event+0xad/0xb2
  [<c075ee8c>] netif_carrier_on+0x28/0x39
  [<f88c34f9>] __rtl8169_check_link_status+0x54/0xb4 [r8169]
  [<f88c3e22>] rtl8169_interrupt+0x1f4/0x298 [r8169]
  [<c0483fe0>] handle_irq_event_percpu+0x58/0x17b
  [<c0484134>] handle_irq_event+0x31/0x48
  [<c0485c2f>] ? handle_percpu_irq+0x40/0x40
  [<c0485cbe>] handle_edge_irq+0x8f/0xb1
  <IRQ>  [<c0403afe>] ? do_IRQ+0x3c/0x95
  [<c07f592e>] ? common_interrupt+0x2e/0x40
  [<c04b00d8>] ? get_page_from_freelist+0x1ab/0x3c9
  [<c04b0bd6>] ? __alloc_pages_nodemask+0x60e/0x66f
  [<c05b6061>] ? blk_finish_plug+0x12/0x2d
  [<c04254b1>] ? pte_alloc_one+0x1c/0x37
  [<c04c1fb7>] ? __pte_alloc+0x1d/0xf3
  [<c04c425f>] ? handle_mm_fault+0xee/0x150
  [<c04c4518>] ? __get_user_pages+0x257/0x39b
  [<c04c46d5>] ? get_user_pages+0x39/0x40
  [<c04eb064>] ? get_arg_page+0x35/0x8e
  [<c05cfc9c>] ? strnlen_user+0x20/0x3e
  [<c04eb1a8>] ? copy_strings+0xeb/0x1b3
  [<c04eb291>] ? copy_strings_kernel+0x21/0x30
  [<c04eb674>] ? do_execve+0x11d/0x22d
  [<c040830a>] ? sys_execve+0x31/0x54
  [<c07f5452>] ? ptregs_execve+0x12/0x20
  [<c07f535c>] ? sysenter_do_call+0x12/0x38
---[ end trace 5bb67ffe27b66e2e ]---

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply

* Re: [PATCH 05/18] virtio: used event index interface
From: Tom Lendacky @ 2011-05-04 21:56 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, Rusty Russell, Carsten Otte, Christian Borntraeger,
	linux390, Martin Schwidefsky, Heiko Carstens, Shirley Ma, lguest,
	virtualization, netdev, linux-s390, kvm, Krishna Kumar, steved,
	habanero
In-Reply-To: <aacced20b8109a615ee24c1bde6d9f5702850111.1304541919.git.mst@redhat.com>

On Wednesday, May 04, 2011 03:51:09 PM Michael S. Tsirkin wrote:
> Define a new feature bit for the guest to utilize a used_event index
> (like Xen) instead if a flag bit to enable/disable interrupts.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  include/linux/virtio_ring.h |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
> index e4d144b..f5c1b75 100644
> --- a/include/linux/virtio_ring.h
> +++ b/include/linux/virtio_ring.h
> @@ -29,6 +29,10 @@
>  /* We support indirect buffer descriptors */
>  #define VIRTIO_RING_F_INDIRECT_DESC	28
> 
> +/* The Guest publishes the used index for which it expects an interrupt
> + * at the end of the avail ring. Host should ignore the avail->flags
> field. */ +#define VIRTIO_RING_F_USED_EVENT_IDX	29
> +
>  /* Virtio ring descriptors: 16 bytes.  These can chain together via
> "next". */ struct vring_desc {
>  	/* Address (guest-physical). */
> @@ -83,6 +87,7 @@ struct vring {
>   *	__u16 avail_flags;
>   *	__u16 avail_idx;
>   *	__u16 available[num];
> + *	__u16 used_event_idx;
>   *
>   *	// Padding to the next align boundary.
>   *	char pad[];
> @@ -93,6 +98,10 @@ struct vring {
>   *	struct vring_used_elem used[num];
>   * };
>   */
> +/* We publish the used event index at the end of the available ring.
> + * It is at the end for backwards compatibility. */
> +#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
> +
>  static inline void vring_init(struct vring *vr, unsigned int num, void *p,
>  			      unsigned long align)
>  {

You should update the vring_size procedure to account for the extra field at 
the end of the available ring by change the "(2 + num)" to "(3 + num)":
    return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)

Tom

^ permalink raw reply

* Re: [ethtool PATCH 4/4] v5 Add RX packet classification interface
From: Ben Hutchings @ 2011-05-04 21:54 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Dimitris Michailidis, davem@davemloft.net, Kirsher, Jeffrey T,
	netdev@vger.kernel.org
In-Reply-To: <4DC1C018.5090709@intel.com>

On Wed, 2011-05-04 at 14:07 -0700, Alexander Duyck wrote:
> On 5/4/2011 11:45 AM, Ben Hutchings wrote:
[...]
> > Please can you confirm that the location specified for
> > ETHTOOL_SRXCLSRLINS will indeed be used as a priority in case of
> > overlapping filters?
> >
> > Ben.
> >
> 
> The ixgbe approach should be nearly identical in terms of how the 
> priorities are based on the location of the filters.

OK, good.

> The original 
> version from Santwona had the rule manager breaking the rules up into 7 
> sections so that rules that specified fewer fields would be near the end 
> of the list.  I'm pretty sure that was all due to priorities from what I 
> could see in the niu driver since the filters that covered wider ranges 
> were being made lower priority to be matched last.

That would make sense.

> In terms of overloading the get count call, that probably would be the 
> best route in terms of changing rule manager behavior.  The only thing I 
> am having a hard time seeing is how the rule manager would be able to 
> distinguish between low priority and high priority filter rules, or is 
> this something that new keywords would be added to the parser for?

Right, there would have to be keywords to specify that.

> I just put out version 6 of the patches.  Essentially I have reduced the 
> size of the rule manager to being used only on insertion without any 
> rule location specified.  The one thing to keep in mind with this rule 
> manager is that the rule at table size - 1 is always going to be the 
> lowest priority rule.  So if it was reserved for unspecified rules it 
> would be easy to use something like that to achieve an "auto-select" 
> location that the driver could then reassign as rules were added to it.

I don't think any location value within the physical table size should
select this special behaviour.  The special location values for
auto-select (with whatever priority) should be distinct from all the
physical location values.

I still need to review your patches but it sounds like they will be
ready to apply.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [PATCH 0/4] [RFC] virtio-net: Improve small packet performance
From: Michael S. Tsirkin @ 2011-05-04 21:23 UTC (permalink / raw)
  To: Krishna Kumar2; +Cc: davem, eric.dumazet, kvm, netdev, rusty
In-Reply-To: <OFAEA8C363.C13C507B-ON65257886.0051F71F-65257886.0052217F@in.ibm.com>

On Wed, May 04, 2011 at 08:29:44PM +0530, Krishna Kumar2 wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote on 05/04/2011 08:16:22 PM:
> 
> > > A. virtio:
> > >    - Provide a API to get available number of slots.
> > >
> > > B. virtio-net:
> > >    - Remove stop/start txq's and associated callback.
> > >    - Pre-calculate the number of slots needed to transmit
> > >      the skb in xmit_skb and bail out early if enough space
> > >      is not available. My testing shows that 2.5-3% of
> > >      packets are benefited by using this API.
> > >    - Do not drop skbs but instead return TX_BUSY like other
> > >      drivers.
> > >    - When returning EBUSY, set a per-txq variable to indicate
> > >      to dev_queue_xmit() whether to restart xmits on this txq.
> > >
> > > C. net/sched/sch_generic.c:
> > >    Since virtio-net now returns EBUSY, the skb is requeued to
> > >    gso_skb. This allows adding the addional check for restart
> > >    xmits in just the slow-path (the first re-queued packet
> > >    case of dequeue_skb, where it checks for gso_skb) before
> > >    deciding whether to call the driver or not.
> > >
> > > Patch was also tested between two servers with Emulex OneConnect
> > > 10G cards to confirm there is no regression. Though the patch is
> > > an attempt to improve only small packet performance, there was
> > > improvement for 1K, 2K and also 16K both in BW and SD. Results
> > > from Guest -> Remote Host (BW in Mbps) for 1K and 16K I/O sizes:
> > >
> > > ________________________________________________________
> > >          I/O Size: 1K
> > > #   BW1   BW2 (%)      SD1   SD2 (%)
> > > ________________________________________________________
> > > 1   1226   3313 (170.2)   6.6   1.9 (-71.2)
> > > 2   3223   7705 (139.0)   18.0   7.1 (-60.5)
> > > 4   7223   8716 (20.6)   36.5   29.7 (-18.6)
> > > 8   8689   8693 (0)   131.5   123.0 (-6.4)
> > > 16   8059   8285 (2.8)   578.3   506.2 (-12.4)
> > > 32   7758   7955 (2.5)   2281.4   2244.2 (-1.6)
> > > 64   7503   7895 (5.2)   9734.0   9424.4 (-3.1)
> > > 96   7496   7751 (3.4)   21980.9   20169.3 (-8.2)
> > > 128   7389   7741 (4.7)   40467.5   34995.5 (-13.5)
> > > ________________________________________________________
> > > Summary:   BW: 16.2%   SD: -10.2%
> > >
> > > ________________________________________________________
> > >          I/O Size: 16K
> > > #   BW1   BW2 (%)      SD1   SD2 (%)
> > > ________________________________________________________
> > > 1   6684   7019 (5.0)   1.1   1.1 (0)
> > > 2   7674   7196 (-6.2)   5.0   4.8 (-4.0)
> > > 4   7358   8032 (9.1)   21.3   20.4 (-4.2)
> > > 8   7393   8015 (8.4)   82.7   82.0 (-.8)
> > > 16   7958   8366 (5.1)   283.2   310.7 (9.7)
> > > 32   7792   8113 (4.1)   1257.5   1363.0 (8.3)
> > > 64   7673   8040 (4.7)   5723.1   5812.4 (1.5)
> > > 96   7462   7883 (5.6)   12731.8   12119.8 (-4.8)
> > > 128   7338   7800 (6.2)   21331.7   21094.7 (-1.1)
> > > ________________________________________________________
> > > Summary:   BW: 4.6%   SD: -1.5%
> > >
> > > Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> > > ---
> >
> > So IIUC, we delay transmit by an arbitrary value and hope
> > that the host is done with the packets by then?
> 
> Not "hope" exactly. If the device is not ready, then
> the packet is requeued. The main idea is to avoid
> drops/stop/starts, etc.

Yes, I see that, definitely. I guess it's a win if the
interrupt takes at least a jiffy to arrive anyway,
and a loss if not. Is there some reason interrupts
might be delayed until the next jiffy?

> > Interesting.
> >
> > I am currently testing an approach where
> > we tell the host explicitly to interrupt us only after
> > a large part of the queue is empty.
> > With 256 entries in a queue, we should get 1 interrupt per
> > on the order of 100 packets which does not seem like a lot.
> >
> > I can post it, mind testing this?
> 
> Sure.
> 
> - KK

Just posted. Would appreciate feedback.

-- 
MST

^ permalink raw reply

* [PDFv2] virtio-spec: 64 bit features, used/avail event
From: Michael S. Tsirkin @ 2011-05-04 21:17 UTC (permalink / raw)
  To: linux-kernel, qemu-devel, kvm
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, linux-s390,
	netdev, habanero, Rusty Russell, Heiko Carstens, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <20110504203256.GA20819@redhat.com>

People asked for a pdf for a new spec, so here it is:
http://userweb.kernel.org/~mst/virtio-spec-event-idx-v2.pdf

Guest and host implementation can be found here:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net-next-event-idx-v1
git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu-kvm.git virtio-net-event-idx-v1

Description reposted below:

I'm working on a patchset (to follow shortly)
that modified the notificatin hand-off in virtio to be basically
like Xen: each side published an index, the other side only triggers
an event when it crosses that index value
(Xen event indexes start at 1, ours start at 0 for
backward-compatiblity, but that's minor).

Especially for testing, it is very convenient to have
separate feature bits for this change in used and available
ring; since we've run out of bits in the 32 bit field,
I added another 32 bit and bit 31 enables that.

I started with using both flags and indexes in parallel,
but switched to doing either-or: this means we do
not need to tweak memory access ordering as index access just
replaces flags access.

A note on naming: the index replacing avail->flags is named
used_event, the index replacing used->flags is named
avail_event to stress the fact that these actually
point into the other side of the ring:
event is triggered when avail->idx == used->avail_event + 1
and when used->idx == avail->used_event + 1, respectively.

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

-- 
MST

^ permalink raw reply

* Re: [net-next-2.6 0/9][pull request] Intel Wired LAN Driver Update
From: David Miller @ 2011-05-04 21:16 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <1304543684.2998.6.camel@jtkirshe-MOBL1>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 04 May 2011 14:14:44 -0700

> On Wed, 2011-05-04 at 13:59 -0700, David Miller wrote:
>> BTW, Jeff, can I get some kind of time frame on when the ethtool_ops->phys_id
>> conversions/removals for e100, e1000, and igb will be coming my way?
>> 
>> It's starting to be on the order of weeks that I've been waiting for
>> this so that I can apply Stephen Hemminger's patch which gets rid of
>> the old ->phys_id interfaces entirely.
>> 
>> Thanks.
> 
> I have the remaining patches in testing currently.  Should have the
> remaining three patches to you by this weekend (if not sooner).

Thanks!

^ permalink raw reply

* Re: [net-next-2.6 0/9][pull request] Intel Wired LAN Driver Update
From: Jeff Kirsher @ 2011-05-04 21:14 UTC (permalink / raw)
  To: David Miller
  Cc: netdev@vger.kernel.org, gospo@redhat.com, bphilips@novell.com
In-Reply-To: <20110504.135904.229767743.davem@davemloft.net>

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

On Wed, 2011-05-04 at 13:59 -0700, David Miller wrote:
> BTW, Jeff, can I get some kind of time frame on when the ethtool_ops->phys_id
> conversions/removals for e100, e1000, and igb will be coming my way?
> 
> It's starting to be on the order of weeks that I've been waiting for
> this so that I can apply Stephen Hemminger's patch which gets rid of
> the old ->phys_id interfaces entirely.
> 
> Thanks.

I have the remaining patches in testing currently.  Should have the
remaining three patches to you by this weekend (if not sooner).

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [net-next-2.6 PATCH] can: rename can_try_module_get to can_get_proto
From: David Miller @ 2011-05-04 21:08 UTC (permalink / raw)
  To: socketcan-fJ+pQTUTwRTk1uMJSBkQmQ
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4DC1B89A.3080904-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

From: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
Date: Wed, 04 May 2011 22:35:38 +0200

> On 04.05.2011 06:42, Kurt Van Dijck wrote:
>> can: rename can_try_module_get to can_get_proto
>> 
>> can_try_module_get does return a struct can_proto.
>> The name explains what is done in so much detail that a caller
>> may not notice that a struct can_proto is locked/unlocked.
>> 
>> Signed-off-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
> 
> Acked-by: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

Applied.

^ permalink raw reply

* Re: [net-next-2.6 PATCH] can: make struct can_proto const
From: David Miller @ 2011-05-04 21:08 UTC (permalink / raw)
  To: socketcan-fJ+pQTUTwRTk1uMJSBkQmQ
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4DC1B86A.40308-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

From: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
Date: Wed, 04 May 2011 22:34:50 +0200

> On 04.05.2011 06:40, Kurt Van Dijck wrote:
>> commit 53914b67993c724cec585863755c9ebc8446e83b had the
>> same message. That commit did put everything in place but
>> did not make can_proto const itself.
>> 
>> Signed-off-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
>> 
> 
> Acked-by: Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

Applied.

^ permalink raw reply

* Re: [ethtool PATCH 4/4] v5 Add RX packet classification interface
From: Alexander Duyck @ 2011-05-04 21:07 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Dimitris Michailidis, davem@davemloft.net, Kirsher, Jeffrey T,
	netdev@vger.kernel.org
In-Reply-To: <1304534738.2926.74.camel@bwh-desktop>

On 5/4/2011 11:45 AM, Ben Hutchings wrote:
> On Wed, 2011-05-04 at 11:21 -0700, Alexander Duyck wrote:
> [...]
>> Honestly what I would prefer to see is a seperate call added such as an
>> ETHTOOL_GRSCLSRLLOC that we could pass the flow specifier to and perhaps
>> include first/last location call in that and then let the driver return
>> where it wants to drop the rule.
>
> This must not be done as a separate operation because it's racy (in fact
> that's an inherent problem with the rule manager).  In the sfc driver
> (and probably others in future) filters could be inserted for RFS at any
> time.
>
>> That way we can avoid having to create
>> an overly complicated rule manager that can handle all the bizarre rule
>> ordering options that I am sure all the different network devices support.
>
> Right, the rule manager can't implement that.
>
>> The only reason I am not implementing this now is because there aren't
>> any drivers in place that would currently use it.  I figure once cxgb
>> has a means in place of supporting flow classifier rules then Dimitris
>> can add the necessary code to ethtool and the kernel to allow the driver
>> to specify rule locations.  I would prefer to avoid adding features
>> based on speculation of what will be needed and would like to be able to
>> actually see how the features will be used.
>
> If you are going to implement the same interface in ixgbe as in niu
> (modulo bugs), then I have no objection to going ahead with this and
> then adding the option for driver-assigned locations later.
>
> Please can you confirm that the location specified for
> ETHTOOL_SRXCLSRLINS will indeed be used as a priority in case of
> overlapping filters?
>
> Ben.
>

The ixgbe approach should be nearly identical in terms of how the 
priorities are based on the location of the filters.  The original 
version from Santwona had the rule manager breaking the rules up into 7 
sections so that rules that specified fewer fields would be near the end 
of the list.  I'm pretty sure that was all due to priorities from what I 
could see in the niu driver since the filters that covered wider ranges 
were being made lower priority to be matched last.

In terms of overloading the get count call, that probably would be the 
best route in terms of changing rule manager behavior.  The only thing I 
am having a hard time seeing is how the rule manager would be able to 
distinguish between low priority and high priority filter rules, or is 
this something that new keywords would be added to the parser for?

I just put out version 6 of the patches.  Essentially I have reduced the 
size of the rule manager to being used only on insertion without any 
rule location specified.  The one thing to keep in mind with this rule 
manager is that the rule at table size - 1 is always going to be the 
lowest priority rule.  So if it was reserved for unspecified rules it 
would be easy to use something like that to achieve an "auto-select" 
location that the driver could then reassign as rules were added to it.

Thanks,

Alex


^ permalink raw reply

* Re: 2.6.38.2, kernel panic, probably related to framentation handling
From: David Miller @ 2011-05-04 21:05 UTC (permalink / raw)
  To: eric.dumazet; +Cc: denys, netdev
In-Reply-To: <1304539346.32152.81.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 04 May 2011 22:02:26 +0200

> [PATCH] net: ip_expire() must revalidate route
> 
> Commit 4a94445c9a5c (net: Use ip_route_input_noref() in input path)
> added a bug in IP defragmentation handling, in case timeout is fired.
> 
> When a frame is defragmented, we use last skb dst field when building
> final skb. Its dst is valid, since we are in rcu read section.
> 
> But if a timeout occurs, we take first queued fragment to build one ICMP
> TIME EXCEEDED message. Problem is all queued skb have weak dst pointers,
> since we escaped RCU critical section after their queueing. icmp_send()
> might dereference a now freed (and possibly reused) part of memory.
> 
> Calling skb_dst_drop() and ip_route_input_noref() to revalidate route is
> the only possible choice.
> 
> Reported-by: Denys Fedoryshchenko <denys@visp.net.lb>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---

Applied to net-2.6 and queued up for -stable, thanks Eric!

^ permalink raw reply

* Re: [PATCH 1/2] libertas: Convert lbs_pr_<level> to pr_<level>
From: Joe Perches @ 2011-05-04 21:01 UTC (permalink / raw)
  To: Dan Williams
  Cc: John W. Linville, libertas-dev, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <1304537949.1097.8.camel@dcbw.foobar.com>

On Wed, 2011-05-04 at 14:39 -0500, Dan Williams wrote:
> On Mon, 2011-05-02 at 16:49 -0700, Joe Perches wrote:
> > Use the standard pr_<level> functions eases grep a bit.
> > Added a few missing terminating newlines to messages.
> > Coalesced long formats.
> Is there any reason to not put the pr_fmt() definition into 'defs.h'
> instead of C&P at the top of every file?  I don't really care either way
> but that seems cleaner since almost all the libertas files are going to
> use logging.

It has to be before any #include <linux/kernel.h>
or any other #include that uses it.

At some point, all of the uses of

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

should be removed as I intend to have that
or an equivalent become the default.



^ permalink raw reply

* Re: [net-next-2.6 0/9][pull request] Intel Wired LAN Driver Update
From: David Miller @ 2011-05-04 20:59 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <20110504.135621.232749554.davem@davemloft.net>


BTW, Jeff, can I get some kind of time frame on when the ethtool_ops->phys_id
conversions/removals for e100, e1000, and igb will be coming my way?

It's starting to be on the order of weeks that I've been waiting for
this so that I can apply Stephen Hemminger's patch which gets rid of
the old ->phys_id interfaces entirely.

Thanks.

^ permalink raw reply

* Re: [net-next-2.6 0/9][pull request] Intel Wired LAN Driver Update
From: David Miller @ 2011-05-04 20:56 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <1304537441-2056-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed,  4 May 2011 12:30:32 -0700

> The following series contains updates to e100, igb, igbvf, ixgb and ixgbe.
> 
> The following are changes since commit e67f88dd12f610da98ca838822f2c9b4e7c6100e:
>   net: dont hold rtnl mutex during netlink dump callbacks
> and are available in the git repository at:
>   master.kernel.org:/pub/scm/linux/kernel/git/jkirsher/net-next-2.6 master

Pulled, thanks Jeff.

^ permalink raw reply

* Re: [PATCH v4 1/1] can: add pruss CAN driver.
From: Oliver Hartkopp @ 2011-05-04 20:55 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: sachi-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/,
	davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
	Arnd Bergmann, Subhasish Ghosh, nsekhar-l0cyMroinI0, open list,
	CAN NETWORK DRIVERS, Marc Kleine-Budde,
	Netdev-u79uwXL29TY76Z2rM5mHXA, m-watkins-l0cyMroinI0,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <4DC17A31.8070409-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

On 04.05.2011 18:09, Wolfgang Grandegger wrote:

> On 05/04/2011 05:57 PM, Kurt Van Dijck wrote:

>> When doing so, I'd vote for an unlimited(by software) list of hardware filters (id/mask).
>> The hardware must abort when no more filters are available.
> 
> Sounds good and not even to complicated. For the SJA1000 we would just
> allow to set the global mask.

Yes. "unlimited(by software)" was a bit misleading at first reading, as we
should not filter IDs by software in the irq handler. But to create a API that
supports as much HW filters as the hardware provides is a good idea.

>> I think that when using hardware filters, knowing the actual device
>> with it's amount of hardware filters is the least of your problems.
>> Userspace applications that suddenly stop working properly due to
>> hw filters (i.e. some traffic not coming in anymore) will be a major
>> source of bugreports.
> 
> Well, hardware filtering will be off by default and must explicitly be
> set by the user, like for the bitrate setting.

To be correct: By the admin.

The setting of CAN HW filters has a system-wide effect to all users on the
local host. The same effect as we have for the setting of the bitrate. This is
the major difference to the user-configurable per-socket CAN-ID filters that
are provided e.g. by the CAN_RAW socket.

As the current netlink configuration interface for CAN interfaces is not
accessible for standard users also this would be the right place to extend the
netlink interface.

Regards,
Oliver

^ permalink raw reply

* Re: [PATCH] tcp_cubic: limit delayed_ack ratio to prevent divide error
From: Brandeburg, Jesse @ 2011-05-04 20:53 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David Miller, Sangtae Ha, Injong Rhee, Valdis.Kletnieks@vt.edu,
	rdunlap@xenotime.net, lkml@techboom.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20110504130456.425dee68@nehalam>



On Wed, 4 May 2011, Stephen Hemminger wrote:

> TCP Cubic keeps a metric that estimates the amount of delayed
> acknowledgements to use in adjusting the window. If an abnormally
> large number of packets are acknowledged at once, then the update
> could wrap and reach zero. This kind of ACK could only
> happen when there was a large window and huge number of
> ACK's were lost.
> 
> This patch limits the value of delayed ack ratio. The choice of 32
> is just a conservative value since normally it should be range of 
> 1 to 4 packets.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

patch seems fine, but please credit the reporter (lkml@techboom.com) with 
reporting the issue with logs, maybe even with Reported-by: and some kind 
of reference to the panic message or the email thread in the text or 
header?


^ permalink raw reply

* [PATCH 15/18] virtio_net: delay TX callbacks
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Rusty Russell, Carsten Otte, Christian Borntraeger, linux390,
	Martin Schwidefsky, Heiko Carstens, Shirley Ma, lguest,
	linux-kernel, virtualization, netdev, linux-s390, kvm,
	Krishna Kumar, Tom Lendacky, steved, habanero
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Ask for delayed callbacks on TX ring full, to give the
other side more of a chance to make progress.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0cb0b06..f685324 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -609,7 +609,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 * before it gets out of hand.  Naturally, this wastes entries. */
 	if (capacity < 2+MAX_SKB_FRAGS) {
 		netif_stop_queue(dev);
-		if (unlikely(!virtqueue_enable_cb(vi->svq))) {
+		if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
 			/* More just got used, free them then recheck. */
 			capacity += free_old_xmit_skbs(vi);
 			if (capacity >= 2+MAX_SKB_FRAGS) {
-- 
1.7.5.53.gc233e


^ permalink raw reply related

* [PATCH 18/18] virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-05-04 20:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

Current code might introduce a lot of latency variation
if there are many pending bufs at the time we
attempt to transmit a new one. This is bad for
real-time applications and can't be good for TCP either.

Free up just enough to both clean up all buffers
eventually and to be able to xmit the next packet.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f33c92b..9982bd7 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -509,17 +509,23 @@ again:
 	return received;
 }
 
-static void free_old_xmit_skbs(struct virtnet_info *vi)
+static bool free_old_xmit_skbs(struct virtnet_info *vi, int capacity)
 {
 	struct sk_buff *skb;
 	unsigned int len;
+	bool c;
+	/* We try to free up at least 2 skbs per one sent, so that we'll get
+	 * all of the memory back if they are used fast enough. */
+	int n = 2;
 
-	while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
+	while ((c = virtqueue_get_capacity(vi->svq) >= capacity) && --n > 0 &&
+	       (skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
 		pr_debug("Sent skb %p\n", skb);
 		vi->dev->stats.tx_bytes += skb->len;
 		vi->dev->stats.tx_packets++;
 		dev_kfree_skb_any(skb);
 	}
+	return c;
 }
 
 static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -574,8 +580,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct virtnet_info *vi = netdev_priv(dev);
 	int capacity;
 
-	/* Free up any pending old buffers before queueing new ones. */
-	free_old_xmit_skbs(vi);
+	/* Free enough pending old buffers to enable queueing new ones. */
+	free_old_xmit_skbs(vi, 2+MAX_SKB_FRAGS);
 
 	/* Try to transmit */
 	capacity = xmit_skb(vi, skb);
@@ -609,9 +615,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 		netif_stop_queue(dev);
 		if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
 			/* More just got used, free them then recheck. */
-			free_old_xmit_skbs(vi);
-			capacity = virtqueue_get_capacity(vi->svq);
-			if (capacity >= 2+MAX_SKB_FRAGS) {
+			if (!likely(free_old_xmit_skbs(vi, 2+MAX_SKB_FRAGS))) {
 				netif_start_queue(dev);
 				virtqueue_disable_cb(vi->svq);
 			}
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 17/18] virtio_net: fix TX capacity checks using new API
From: Michael S. Tsirkin @ 2011-05-04 20:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

virtio net uses the number of sg entries to
check for TX ring capacity freed. But this
gives incorrect results when indirect buffers
are used. Use the new capacity API instead.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f685324..f33c92b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -509,19 +509,17 @@ again:
 	return received;
 }
 
-static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
+static void free_old_xmit_skbs(struct virtnet_info *vi)
 {
 	struct sk_buff *skb;
-	unsigned int len, tot_sgs = 0;
+	unsigned int len;
 
 	while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
 		pr_debug("Sent skb %p\n", skb);
 		vi->dev->stats.tx_bytes += skb->len;
 		vi->dev->stats.tx_packets++;
-		tot_sgs += skb_vnet_hdr(skb)->num_sg;
 		dev_kfree_skb_any(skb);
 	}
-	return tot_sgs;
 }
 
 static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -611,7 +609,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 		netif_stop_queue(dev);
 		if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
 			/* More just got used, free them then recheck. */
-			capacity += free_old_xmit_skbs(vi);
+			free_old_xmit_skbs(vi);
+			capacity = virtqueue_get_capacity(vi->svq);
 			if (capacity >= 2+MAX_SKB_FRAGS) {
 				netif_start_queue(dev);
 				virtqueue_disable_cb(vi->svq);
-- 
1.7.5.53.gc233e

^ permalink raw reply related

* [PATCH 16/18] virtio_ring: Add capacity check API
From: Michael S. Tsirkin @ 2011-05-04 20:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
	netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
	steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
	linux390
In-Reply-To: <cover.1304541918.git.mst@redhat.com>

>From: Shirley Ma <mashirle@us.ibm.com>

Signed-off-by: Shirley Ma <xma@us.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

I'm not sure who wrote this first anymore :)
But it's a simple patch.

 drivers/virtio/virtio_ring.c |    8 ++++++++
 include/linux/virtio.h       |    5 +++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 3a70d70..57bf9d5 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -365,6 +365,14 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
 }
 EXPORT_SYMBOL_GPL(virtqueue_get_buf);
 
+int virtqueue_get_capacity(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	return vq->num_free;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_capacity);
+
 void virtqueue_disable_cb(struct virtqueue *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 5151fd1..944ebcd 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -42,6 +42,9 @@ struct virtqueue {
  *	vq: the struct virtqueue we're talking about.
  *	len: the length written into the buffer
  *	Returns NULL or the "data" token handed to add_buf.
+ * virtqueue_get_capacity: get the current capacity of the queue
+ *	vq: the struct virtqueue we're talking about.
+ *	Returns remaining capacity of the queue.
  * virtqueue_disable_cb: disable callbacks
  *	vq: the struct virtqueue we're talking about.
  *	Note that this is not necessarily synchronous, hence unreliable and only
@@ -89,6 +92,8 @@ void virtqueue_kick(struct virtqueue *vq);
 
 void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len);
 
+int virtqueue_get_capacity(struct virtqueue *vq);
+
 void virtqueue_disable_cb(struct virtqueue *vq);
 
 bool virtqueue_enable_cb(struct virtqueue *vq);
-- 
1.7.5.53.gc233e

^ permalink raw reply related


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