[PATCH 0/7] staging: vt6656: clean-up error path on init

[PATCH 0/7] staging: vt6656: clean-up error path on init

[Quentin Deslandes]

This patchset aims to cleanup vt6656 driver's error path during the
initialization process.

During a call to vnt_start(), none of the functions called would return
a meaningful error code nor handle the one returned from the functions
they call themselves.

The first patch of the series fixes a potential NULL pointer dereferencing.
All the other patches update function's error management workflow and prototype
when needed.

More functions would need to be updated, but focusing on initialization
process force to change only a reasonable amount of code.

Thank you,
Quentin

Quentin Deslandes (7):
  staging: vt6656: fix potential NULL pointer dereference
  staging: vt6656: clean function's error path in usbpipe.c
  staging: vt6656: avoid discarding called function's return code
  staging: vt6656: clean error path for firmware management
  staging: vt6656: use meaningful error code during buffer allocation
  staging: vt6656: clean-up registers initialization error path
  staging: vt6656: manage error path during device initialization

 drivers/staging/vt6656/baseband.c | 130 +++++++++++------
 drivers/staging/vt6656/baseband.h |   8 +-
 drivers/staging/vt6656/card.c     |  20 ++-
 drivers/staging/vt6656/firmware.c |  91 ++++++------
 drivers/staging/vt6656/int.c      |   8 +-
 drivers/staging/vt6656/int.h      |   2 +-
 drivers/staging/vt6656/mac.c      |  19 ++-
 drivers/staging/vt6656/mac.h      |   6 +-
 drivers/staging/vt6656/main_usb.c | 230 ++++++++++++++++++------------
 drivers/staging/vt6656/rf.c       |  38 +++--
 drivers/staging/vt6656/rf.h       |   2 +-
 drivers/staging/vt6656/usbpipe.c  | 115 ++++++++-------
 drivers/staging/vt6656/usbpipe.h  |   4 +-
 13 files changed, 400 insertions(+), 273 deletions(-)

-- 
2.17.1

[PATCH 1/7] staging: vt6656: fix potential NULL pointer dereference

[Quentin Deslandes]

vnt_free_tx_bufs() relies on priv->tx_context elements to be NULL if
they are not initialized (as vnt_free_rx_bufs() does). Add a check to
these elements in order to avoid NULL pointer dereference.

Signed-off-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk>
---
 drivers/staging/vt6656/main_usb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
index ccafcc2c87ac..bfe952fe27bf 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -363,6 +363,9 @@ static void vnt_free_tx_bufs(struct vnt_private *priv)
 
 	for (ii = 0; ii < priv->num_tx_context; ii++) {
 		tx_context = priv->tx_context[ii];
+		if (!tx_context)
+			continue;
+
 		/* deallocate URBs */
 		if (tx_context->urb) {
 			usb_kill_urb(tx_context->urb);
-- 
2.17.1

[PATCH 2/7] staging: vt6656: clean function's error path in usbpipe.c

[Quentin Deslandes]

Avoid discarding called function's returned value. Store it instead in
order to act accordingly.

Update error path to return 0 on success and a negative errno value on
error.

Signed-off-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk>
---
 drivers/staging/vt6656/usbpipe.c | 115 +++++++++++++++++--------------
 drivers/staging/vt6656/usbpipe.h |   4 +-
 2 files changed, 67 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/vt6656/usbpipe.c b/drivers/staging/vt6656/usbpipe.c
index 5bbc56f8779e..ff351a7a0876 100644
--- a/drivers/staging/vt6656/usbpipe.c
+++ b/drivers/staging/vt6656/usbpipe.c
@@ -36,80 +36,86 @@
 int vnt_control_out(struct vnt_private *priv, u8 request, u16 value,
 		    u16 index, u16 length, u8 *buffer)
 {
-	int status = 0;
+	int ret = 0;
 	u8 *usb_buffer;
 
-	if (test_bit(DEVICE_FLAGS_DISCONNECTED, &priv->flags))
-		return STATUS_FAILURE;
+	if (test_bit(DEVICE_FLAGS_DISCONNECTED, &priv->flags)) {
+		ret = -EINVAL;
+		goto end;
+	}
 
 	mutex_lock(&priv->usb_lock);
 
 	usb_buffer = kmemdup(buffer, length, GFP_KERNEL);
 	if (!usb_buffer) {
-		mutex_unlock(&priv->usb_lock);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto end_unlock;
 	}
 
-	status = usb_control_msg(priv->usb,
-				 usb_sndctrlpipe(priv->usb, 0),
-				 request, 0x40, value,
-				 index, usb_buffer, length, USB_CTL_WAIT);
+	ret = usb_control_msg(priv->usb,
+			      usb_sndctrlpipe(priv->usb, 0),
+			      request, 0x40, value,
+			      index, usb_buffer, length, USB_CTL_WAIT);
 
 	kfree(usb_buffer);
 
-	mutex_unlock(&priv->usb_lock);
+	if (ret >= 0 && ret < (int)length)
+		ret = -EIO;
 
-	if (status < (int)length)
-		return STATUS_FAILURE;
-
-	return STATUS_SUCCESS;
+end_unlock:
+	mutex_unlock(&priv->usb_lock);
+end:
+	return ret;
 }
 
-void vnt_control_out_u8(struct vnt_private *priv, u8 reg, u8 reg_off, u8 data)
+int vnt_control_out_u8(struct vnt_private *priv, u8 reg, u8 reg_off, u8 data)
 {
-	vnt_control_out(priv, MESSAGE_TYPE_WRITE,
-			reg_off, reg, sizeof(u8), &data);
+	return vnt_control_out(priv, MESSAGE_TYPE_WRITE,
+			       reg_off, reg, sizeof(u8), &data);
 }
 
 int vnt_control_in(struct vnt_private *priv, u8 request, u16 value,
 		   u16 index, u16 length, u8 *buffer)
 {
-	int status;
+	int ret = 0;
 	u8 *usb_buffer;
 
-	if (test_bit(DEVICE_FLAGS_DISCONNECTED, &priv->flags))
-		return STATUS_FAILURE;
+	if (test_bit(DEVICE_FLAGS_DISCONNECTED, &priv->flags)) {
+		ret = -EINVAL;
+		goto end;
+	}
 
 	mutex_lock(&priv->usb_lock);
 
 	usb_buffer = kmalloc(length, GFP_KERNEL);
 	if (!usb_buffer) {
-		mutex_unlock(&priv->usb_lock);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto end_unlock;
 	}
 
-	status = usb_control_msg(priv->usb,
-				 usb_rcvctrlpipe(priv->usb, 0),
-				 request, 0xc0, value,
-				 index, usb_buffer, length, USB_CTL_WAIT);
+	ret = usb_control_msg(priv->usb,
+			      usb_rcvctrlpipe(priv->usb, 0),
+			      request, 0xc0, value,
+			      index, usb_buffer, length, USB_CTL_WAIT);
 
-	if (status == length)
+	if (ret == length)
 		memcpy(buffer, usb_buffer, length);
 
 	kfree(usb_buffer);
 
-	mutex_unlock(&priv->usb_lock);
+	if (ret >= 0 && ret < (int)length)
+		ret = -EIO;
 
-	if (status < (int)length)
-		return STATUS_FAILURE;
-
-	return STATUS_SUCCESS;
+end_unlock:
+	mutex_unlock(&priv->usb_lock);
+end:
+	return ret;
 }
 
-void vnt_control_in_u8(struct vnt_private *priv, u8 reg, u8 reg_off, u8 *data)
+int vnt_control_in_u8(struct vnt_private *priv, u8 reg, u8 reg_off, u8 *data)
 {
-	vnt_control_in(priv, MESSAGE_TYPE_READ,
-		       reg_off, reg, sizeof(u8), data);
+	return vnt_control_in(priv, MESSAGE_TYPE_READ,
+			      reg_off, reg, sizeof(u8), data);
 }
 
 static void vnt_start_interrupt_urb_complete(struct urb *urb)
@@ -147,10 +153,12 @@ static void vnt_start_interrupt_urb_complete(struct urb *urb)
 
 int vnt_start_interrupt_urb(struct vnt_private *priv)
 {
-	int status = STATUS_FAILURE;
+	int ret = 0;
 
-	if (priv->int_buf.in_use)
-		return STATUS_FAILURE;
+	if (priv->int_buf.in_use) {
+		ret = -EBUSY;
+		goto err;
+	}
 
 	priv->int_buf.in_use = true;
 
@@ -163,13 +171,18 @@ int vnt_start_interrupt_urb(struct vnt_private *priv)
 			 priv,
 			 priv->int_interval);
 
-	status = usb_submit_urb(priv->interrupt_urb, GFP_ATOMIC);
-	if (status) {
-		dev_dbg(&priv->usb->dev, "Submit int URB failed %d\n", status);
-		priv->int_buf.in_use = false;
+	ret = usb_submit_urb(priv->interrupt_urb, GFP_ATOMIC);
+	if (ret) {
+		dev_dbg(&priv->usb->dev, "Submit int URB failed %d\n", ret);
+		goto err_submit;
 	}
 
-	return status;
+	return 0;
+
+err_submit:
+	priv->int_buf.in_use = false;
+err:
+	return ret;
 }
 
 static void vnt_submit_rx_urb_complete(struct urb *urb)
@@ -215,12 +228,13 @@ static void vnt_submit_rx_urb_complete(struct urb *urb)
 
 int vnt_submit_rx_urb(struct vnt_private *priv, struct vnt_rcb *rcb)
 {
-	int status = 0;
+	int ret = 0;
 	struct urb *urb = rcb->urb;
 
 	if (!rcb->skb) {
 		dev_dbg(&priv->usb->dev, "rcb->skb is null\n");
-		return status;
+		ret = -EINVAL;
+		goto end;
 	}
 
 	usb_fill_bulk_urb(urb,
@@ -231,15 +245,16 @@ int vnt_submit_rx_urb(struct vnt_private *priv, struct vnt_rcb *rcb)
 			  vnt_submit_rx_urb_complete,
 			  rcb);
 
-	status = usb_submit_urb(urb, GFP_ATOMIC);
-	if (status) {
-		dev_dbg(&priv->usb->dev, "Submit Rx URB failed %d\n", status);
-		return STATUS_FAILURE;
+	ret = usb_submit_urb(urb, GFP_ATOMIC);
+	if (ret) {
+		dev_dbg(&priv->usb->dev, "Submit Rx URB failed %d\n", ret);
+		goto end;
 	}
 
 	rcb->in_use = true;
 
-	return status;
+end:
+	return ret;
 }
 
 static void vnt_tx_context_complete(struct urb *urb)
diff --git a/drivers/staging/vt6656/usbpipe.h b/drivers/staging/vt6656/usbpipe.h
index 2910ca54886e..95147ec7b96a 100644
--- a/drivers/staging/vt6656/usbpipe.h
+++ b/drivers/staging/vt6656/usbpipe.h
@@ -23,8 +23,8 @@ int vnt_control_out(struct vnt_private *priv, u8 request, u16 value,
 int vnt_control_in(struct vnt_private *priv, u8 request, u16 value,
 		   u16 index, u16 length,  u8 *buffer);
 
-void vnt_control_out_u8(struct vnt_private *priv, u8 reg, u8 ref_off, u8 data);
-void vnt_control_in_u8(struct vnt_private *priv, u8 reg, u8 reg_off, u8 *data);
+int vnt_control_out_u8(struct vnt_private *priv, u8 reg, u8 ref_off, u8 data);
+int vnt_control_in_u8(struct vnt_private *priv, u8 reg, u8 reg_off, u8 *data);
 
 int vnt_start_interrupt_urb(struct vnt_private *priv);
 int vnt_submit_rx_urb(struct vnt_private *priv, struct vnt_rcb *rcb);
-- 
2.17.1

[PATCH 3/7] staging: vt6656: avoid discarding called function's return code

[Quentin Deslandes]

Change some of the driver's functions in order to handle error codes
instead of discarding them. These function now returns 0 on success and
a negative errno value on error.

Signed-off-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk>
---
 drivers/staging/vt6656/baseband.c | 130 ++++++++++++++++++++----------
 drivers/staging/vt6656/baseband.h |   8 +-
 drivers/staging/vt6656/card.c     |  20 +++--
 drivers/staging/vt6656/int.c      |   8 +-
 drivers/staging/vt6656/int.h      |   2 +-
 drivers/staging/vt6656/mac.c      |  19 +++--
 drivers/staging/vt6656/mac.h      |   6 +-
 drivers/staging/vt6656/rf.c       |  38 ++++++---
 drivers/staging/vt6656/rf.h       |   2 +-
 9 files changed, 152 insertions(+), 81 deletions(-)

diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
index b29ba237fa29..8d19ae71e7cc 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -329,7 +329,7 @@ void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length,
  * Return Value: none
  *
  */
-void vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode)
+int vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode)
 {
 	switch (antenna_mode) {
 	case ANT_TXA:
@@ -344,8 +344,8 @@ void vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode)
 		break;
 	}
 
-	vnt_control_out(priv, MESSAGE_TYPE_SET_ANTMD,
-			(u16)antenna_mode, 0, 0, NULL);
+	return vnt_control_out(priv, MESSAGE_TYPE_SET_ANTMD,
+			       (u16)antenna_mode, 0, 0, NULL);
 }
 
 /*
@@ -364,7 +364,7 @@ void vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode)
 
 int vnt_vt3184_init(struct vnt_private *priv)
 {
-	int status;
+	int ret = 0;
 	u16 length;
 	u8 *addr;
 	u8 *agc;
@@ -372,11 +372,10 @@ int vnt_vt3184_init(struct vnt_private *priv)
 	u8 array[256];
 	u8 data;
 
-	status = vnt_control_in(priv, MESSAGE_TYPE_READ, 0,
-				MESSAGE_REQUEST_EEPROM, EEP_MAX_CONTEXT_SIZE,
-						priv->eeprom);
-	if (status != STATUS_SUCCESS)
-		return false;
+	ret = vnt_control_in(priv, MESSAGE_TYPE_READ, 0, MESSAGE_REQUEST_EEPROM,
+			     EEP_MAX_CONTEXT_SIZE, priv->eeprom);
+	if (ret)
+		goto end;
 
 	priv->rf_type = priv->eeprom[EEP_OFS_RFTYPE];
 
@@ -423,8 +422,10 @@ int vnt_vt3184_init(struct vnt_private *priv)
 		priv->bb_vga[3] = 0x0;
 
 		/* Fix VT3226 DFC system timing issue */
-		vnt_mac_reg_bits_on(priv, MAC_REG_SOFTPWRCTL2,
-				    SOFTPWRCTL_RFLEOPT);
+		ret = vnt_mac_reg_bits_on(priv, MAC_REG_SOFTPWRCTL2,
+					  SOFTPWRCTL_RFLEOPT);
+		if (ret)
+			goto end;
 	} else if (priv->rf_type == RF_VT3342A0) {
 		priv->bb_rx_conf = vnt_vt3184_vt3226d0[10];
 		length = sizeof(vnt_vt3184_vt3226d0);
@@ -438,48 +439,74 @@ int vnt_vt3184_init(struct vnt_private *priv)
 		priv->bb_vga[3] = 0x0;
 
 		/* Fix VT3226 DFC system timing issue */
-		vnt_mac_reg_bits_on(priv, MAC_REG_SOFTPWRCTL2,
-				    SOFTPWRCTL_RFLEOPT);
+		ret = vnt_mac_reg_bits_on(priv, MAC_REG_SOFTPWRCTL2,
+					  SOFTPWRCTL_RFLEOPT);
+		if (ret)
+			goto end;
 	} else {
-		return true;
+		goto end;
 	}
 
 	memcpy(array, addr, length);
 
-	vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
-			MESSAGE_REQUEST_BBREG, length, array);
+	ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
+			      MESSAGE_REQUEST_BBREG, length, array);
+	if (ret)
+		goto end;
 
 	memcpy(array, agc, length_agc);
 
-	vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
-			MESSAGE_REQUEST_BBAGC, length_agc, array);
+	ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
+			      MESSAGE_REQUEST_BBAGC, length_agc, array);
+	if (ret)
+		goto end;
 
 	if ((priv->rf_type == RF_VT3226) ||
 	    (priv->rf_type == RF_VT3342A0)) {
-		vnt_control_out_u8(priv, MESSAGE_REQUEST_MACREG,
-				   MAC_REG_ITRTMSET, 0x23);
-		vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, 0x01);
+		ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_MACREG,
+					 MAC_REG_ITRTMSET, 0x23);
+		if (ret)
+			goto end;
+
+		ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, 0x01);
+		if (ret)
+			goto end;
 	} else if (priv->rf_type == RF_VT3226D0) {
-		vnt_control_out_u8(priv, MESSAGE_REQUEST_MACREG,
-				   MAC_REG_ITRTMSET, 0x11);
-		vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, 0x01);
+		ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_MACREG,
+					 MAC_REG_ITRTMSET, 0x11);
+		if (ret)
+			goto end;
+
+		ret = vnt_mac_reg_bits_on(priv, MAC_REG_PAPEDELAY, 0x01);
+		if (ret)
+			goto end;
 	}
 
-	vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x04, 0x7f);
-	vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0d, 0x01);
+	ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x04, 0x7f);
+	if (ret)
+		goto end;
 
-	vnt_rf_table_download(priv);
+	ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0d, 0x01);
+	if (ret)
+		goto end;
+
+	ret = vnt_rf_table_download(priv);
+	if (ret)
+		goto end;
 
 	/* Fix for TX USB resets from vendors driver */
-	vnt_control_in(priv, MESSAGE_TYPE_READ, USB_REG4,
-		       MESSAGE_REQUEST_MEM, sizeof(data), &data);
+	ret = vnt_control_in(priv, MESSAGE_TYPE_READ, USB_REG4,
+			     MESSAGE_REQUEST_MEM, sizeof(data), &data);
+	if (ret)
+		goto end;
 
 	data |= 0x2;
 
-	vnt_control_out(priv, MESSAGE_TYPE_WRITE, USB_REG4,
-			MESSAGE_REQUEST_MEM, sizeof(data), &data);
+	ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, USB_REG4,
+			      MESSAGE_REQUEST_MEM, sizeof(data), &data);
 
-	return true;
+end:
+	return ret;
 }
 
 /*
@@ -494,8 +521,9 @@ int vnt_vt3184_init(struct vnt_private *priv)
  * Return Value: none
  *
  */
-void vnt_set_short_slot_time(struct vnt_private *priv)
+int vnt_set_short_slot_time(struct vnt_private *priv)
 {
+	int ret = 0;
 	u8 bb_vga = 0;
 
 	if (priv->short_slot_time)
@@ -503,12 +531,18 @@ void vnt_set_short_slot_time(struct vnt_private *priv)
 	else
 		priv->bb_rx_conf |= 0x20;
 
-	vnt_control_in_u8(priv, MESSAGE_REQUEST_BBREG, 0xe7, &bb_vga);
+	ret = vnt_control_in_u8(priv, MESSAGE_REQUEST_BBREG, 0xe7, &bb_vga);
+	if (ret)
+		goto end;
 
 	if (bb_vga == priv->bb_vga[0])
 		priv->bb_rx_conf |= 0x20;
 
-	vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a, priv->bb_rx_conf);
+	ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0a,
+				 priv->bb_rx_conf);
+
+end:
+	return ret;
 }
 
 void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data)
@@ -536,16 +570,30 @@ void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data)
  * Return Value: none
  *
  */
-void vnt_set_deep_sleep(struct vnt_private *priv)
+int vnt_set_deep_sleep(struct vnt_private *priv)
 {
-	vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0c, 0x17);/* CR12 */
-	vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0d, 0xB9);/* CR13 */
+	int ret = 0;
+
+	/* CR12 */
+	ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0c, 0x17);
+	if (ret)
+		return ret;
+
+	/* CR13 */
+	return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0d, 0xB9);
 }
 
-void vnt_exit_deep_sleep(struct vnt_private *priv)
+int vnt_exit_deep_sleep(struct vnt_private *priv)
 {
-	vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0c, 0x00);/* CR12 */
-	vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0d, 0x01);/* CR13 */
+	int ret = 0;
+
+	/* CR12 */
+	ret = vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0c, 0x00);
+	if (ret)
+		return ret;
+
+	/* CR13 */
+	return vnt_control_out_u8(priv, MESSAGE_REQUEST_BBREG, 0x0d, 0x01);
 }
 
 void vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning)
diff --git a/drivers/staging/vt6656/baseband.h b/drivers/staging/vt6656/baseband.h
index c3b8bbdb3ea1..dc42aa6ae1d9 100644
--- a/drivers/staging/vt6656/baseband.h
+++ b/drivers/staging/vt6656/baseband.h
@@ -79,12 +79,12 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
 void vnt_get_phy_field(struct vnt_private *priv, u32 frame_length,
 		       u16 tx_rate, u8 pkt_type, struct vnt_phy_field *phy);
 
-void vnt_set_short_slot_time(struct vnt_private *priv);
+int vnt_set_short_slot_time(struct vnt_private *priv);
 void vnt_set_vga_gain_offset(struct vnt_private *priv, u8 data);
-void vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode);
+int vnt_set_antenna_mode(struct vnt_private *priv, u8 antenna_mode);
 int vnt_vt3184_init(struct vnt_private *priv);
-void vnt_set_deep_sleep(struct vnt_private *priv);
-void vnt_exit_deep_sleep(struct vnt_private *priv);
+int vnt_set_deep_sleep(struct vnt_private *priv);
+int vnt_exit_deep_sleep(struct vnt_private *priv);
 void vnt_update_pre_ed_threshold(struct vnt_private *priv, int scanning);
 
 #endif /* __BASEBAND_H__ */
diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
index 501f482b41c4..08fc03d8740e 100644
--- a/drivers/staging/vt6656/card.c
+++ b/drivers/staging/vt6656/card.c
@@ -674,7 +674,7 @@ void vnt_update_next_tbtt(struct vnt_private *priv, u64 tsf,
  */
 int vnt_radio_power_off(struct vnt_private *priv)
 {
-	int ret = true;
+	int ret = 0;
 
 	switch (priv->rf_type) {
 	case RF_AL2230:
@@ -683,17 +683,25 @@ int vnt_radio_power_off(struct vnt_private *priv)
 	case RF_VT3226:
 	case RF_VT3226D0:
 	case RF_VT3342A0:
-		vnt_mac_reg_bits_off(priv, MAC_REG_SOFTPWRCTL,
-				     (SOFTPWRCTL_SWPE2 | SOFTPWRCTL_SWPE3));
+		ret = vnt_mac_reg_bits_off(priv, MAC_REG_SOFTPWRCTL,
+					(SOFTPWRCTL_SWPE2 | SOFTPWRCTL_SWPE3));
 		break;
 	}
 
-	vnt_mac_reg_bits_off(priv, MAC_REG_HOSTCR, HOSTCR_RXON);
+	if (ret)
+		goto end;
+
+	ret = vnt_mac_reg_bits_off(priv, MAC_REG_HOSTCR, HOSTCR_RXON);
+	if (ret)
+		goto end;
 
-	vnt_set_deep_sleep(priv);
+	ret = vnt_set_deep_sleep(priv);
+	if (ret)
+		goto end;
 
-	vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
+	ret = vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL1, GPIO3_INTMD);
 
+end:
 	return ret;
 }
 
diff --git a/drivers/staging/vt6656/int.c b/drivers/staging/vt6656/int.c
index 504424b19fcf..f40947955675 100644
--- a/drivers/staging/vt6656/int.c
+++ b/drivers/staging/vt6656/int.c
@@ -39,18 +39,20 @@ static const u8 fallback_rate1[5][5] = {
 	{RATE_54M, RATE_54M, RATE_36M, RATE_18M, RATE_18M}
 };
 
-void vnt_int_start_interrupt(struct vnt_private *priv)
+int vnt_int_start_interrupt(struct vnt_private *priv)
 {
+	int ret = 0;
 	unsigned long flags;
-	int status;
 
 	dev_dbg(&priv->usb->dev, "---->Interrupt Polling Thread\n");
 
 	spin_lock_irqsave(&priv->lock, flags);
 
-	status = vnt_start_interrupt_urb(priv);
+	ret = vnt_start_interrupt_urb(priv);
 
 	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return ret;
 }
 
 static int vnt_int_report_rate(struct vnt_private *priv, u8 pkt_no, u8 tsr)
diff --git a/drivers/staging/vt6656/int.h b/drivers/staging/vt6656/int.h
index 987c454e99e9..8a6d60569ceb 100644
--- a/drivers/staging/vt6656/int.h
+++ b/drivers/staging/vt6656/int.h
@@ -41,7 +41,7 @@ struct vnt_interrupt_data {
 	u8 sw[2];
 } __packed;
 
-void vnt_int_start_interrupt(struct vnt_private *priv);
+int vnt_int_start_interrupt(struct vnt_private *priv);
 void vnt_int_process_data(struct vnt_private *priv);
 
 #endif /* __INT_H__ */
diff --git a/drivers/staging/vt6656/mac.c b/drivers/staging/vt6656/mac.c
index 0b543854ea97..5cacf6e60e90 100644
--- a/drivers/staging/vt6656/mac.c
+++ b/drivers/staging/vt6656/mac.c
@@ -129,27 +129,26 @@ void vnt_mac_set_keyentry(struct vnt_private *priv, u16 key_ctl, u32 entry_idx,
 			(u8 *)&set_key);
 }
 
-void vnt_mac_reg_bits_off(struct vnt_private *priv, u8 reg_ofs, u8 bits)
+int vnt_mac_reg_bits_off(struct vnt_private *priv, u8 reg_ofs, u8 bits)
 {
 	u8 data[2];
 
 	data[0] = 0;
 	data[1] = bits;
 
-	vnt_control_out(priv, MESSAGE_TYPE_WRITE_MASK,
-			reg_ofs, MESSAGE_REQUEST_MACREG, ARRAY_SIZE(data),
-			data);
+	return vnt_control_out(priv, MESSAGE_TYPE_WRITE_MASK, reg_ofs,
+			       MESSAGE_REQUEST_MACREG, ARRAY_SIZE(data), data);
 }
 
-void vnt_mac_reg_bits_on(struct vnt_private *priv, u8 reg_ofs, u8 bits)
+int vnt_mac_reg_bits_on(struct vnt_private *priv, u8 reg_ofs, u8 bits)
 {
 	u8 data[2];
 
 	data[0] = bits;
 	data[1] = bits;
 
-	vnt_control_out(priv, MESSAGE_TYPE_WRITE_MASK, reg_ofs,
-			MESSAGE_REQUEST_MACREG, ARRAY_SIZE(data), data);
+	return vnt_control_out(priv, MESSAGE_TYPE_WRITE_MASK, reg_ofs,
+			       MESSAGE_REQUEST_MACREG, ARRAY_SIZE(data), data);
 }
 
 void vnt_mac_write_word(struct vnt_private *priv, u8 reg_ofs, u16 word)
@@ -224,13 +223,13 @@ void vnt_mac_set_beacon_interval(struct vnt_private *priv, u16 interval)
 			MESSAGE_REQUEST_MACREG, ARRAY_SIZE(data), data);
 }
 
-void vnt_mac_set_led(struct vnt_private *priv, u8 state, u8 led)
+int vnt_mac_set_led(struct vnt_private *priv, u8 state, u8 led)
 {
 	u8 data[2];
 
 	data[0] = led;
 	data[1] = state;
 
-	vnt_control_out(priv, MESSAGE_TYPE_WRITE_MASK, MAC_REG_PAPEDELAY,
-			MESSAGE_REQUEST_MACREG, ARRAY_SIZE(data), data);
+	return vnt_control_out(priv, MESSAGE_TYPE_WRITE_MASK, MAC_REG_PAPEDELAY,
+			       MESSAGE_REQUEST_MACREG, ARRAY_SIZE(data), data);
 }
diff --git a/drivers/staging/vt6656/mac.h b/drivers/staging/vt6656/mac.h
index 3fd87f95c524..0a42308b81e9 100644
--- a/drivers/staging/vt6656/mac.h
+++ b/drivers/staging/vt6656/mac.h
@@ -360,8 +360,8 @@ void vnt_mac_set_bb_type(struct vnt_private *priv, u8 type);
 void vnt_mac_disable_keyentry(struct vnt_private *priv, u8 entry_idx);
 void vnt_mac_set_keyentry(struct vnt_private *priv, u16 key_ctl, u32 entry_idx,
 			  u32 key_idx, u8 *addr, u8 *key);
-void vnt_mac_reg_bits_off(struct vnt_private *priv, u8 reg_ofs, u8 bits);
-void vnt_mac_reg_bits_on(struct vnt_private *priv, u8 reg_ofs, u8 bits);
+int vnt_mac_reg_bits_off(struct vnt_private *priv, u8 reg_ofs, u8 bits);
+int vnt_mac_reg_bits_on(struct vnt_private *priv, u8 reg_ofs, u8 bits);
 void vnt_mac_write_word(struct vnt_private *priv, u8 reg_ofs, u16 word);
 void vnt_mac_set_bssid_addr(struct vnt_private *priv, u8 *addr);
 void vnt_mac_enable_protect_mode(struct vnt_private *priv);
@@ -369,6 +369,6 @@ void vnt_mac_disable_protect_mode(struct vnt_private *priv);
 void vnt_mac_enable_barker_preamble_mode(struct vnt_private *priv);
 void vnt_mac_disable_barker_preamble_mode(struct vnt_private *priv);
 void vnt_mac_set_beacon_interval(struct vnt_private *priv, u16 interval);
-void vnt_mac_set_led(struct vnt_private *privpriv, u8 state, u8 led);
+int vnt_mac_set_led(struct vnt_private *privpriv, u8 state, u8 led);
 
 #endif /* __MAC_H__ */
diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 18f75dcc65d2..43237b7e1dbe 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -811,8 +811,9 @@ void vnt_rf_rssi_to_dbm(struct vnt_private *priv, u8 rssi, long *dbm)
 	*dbm = -1 * (a + b * 2);
 }
 
-void vnt_rf_table_download(struct vnt_private *priv)
+int vnt_rf_table_download(struct vnt_private *priv)
 {
+	int ret = 0;
 	u16 length1 = 0, length2 = 0, length3 = 0;
 	u8 *addr1 = NULL, *addr2 = NULL, *addr3 = NULL;
 	u16 length, value;
@@ -865,8 +866,10 @@ void vnt_rf_table_download(struct vnt_private *priv)
 	/* Init Table */
 	memcpy(array, addr1, length1);
 
-	vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
-			MESSAGE_REQUEST_RF_INIT, length1, array);
+	ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
+			      MESSAGE_REQUEST_RF_INIT, length1, array);
+	if (ret)
+		goto end;
 
 	/* Channel Table 0 */
 	value = 0;
@@ -878,8 +881,10 @@ void vnt_rf_table_download(struct vnt_private *priv)
 
 		memcpy(array, addr2, length);
 
-		vnt_control_out(priv, MESSAGE_TYPE_WRITE,
-				value, MESSAGE_REQUEST_RF_CH0, length, array);
+		ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, value,
+				      MESSAGE_REQUEST_RF_CH0, length, array);
+		if (ret)
+			goto end;
 
 		length2 -= length;
 		value += length;
@@ -896,8 +901,10 @@ void vnt_rf_table_download(struct vnt_private *priv)
 
 		memcpy(array, addr3, length);
 
-		vnt_control_out(priv, MESSAGE_TYPE_WRITE,
-				value, MESSAGE_REQUEST_RF_CH1, length, array);
+		ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, value,
+				      MESSAGE_REQUEST_RF_CH1, length, array);
+		if (ret)
+			goto end;
 
 		length3 -= length;
 		value += length;
@@ -913,8 +920,10 @@ void vnt_rf_table_download(struct vnt_private *priv)
 		memcpy(array, addr1, length1);
 
 		/* Init Table 2 */
-		vnt_control_out(priv, MESSAGE_TYPE_WRITE,
-				0, MESSAGE_REQUEST_RF_INIT2, length1, array);
+		ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, 0,
+				      MESSAGE_REQUEST_RF_INIT2, length1, array);
+		if (ret)
+			goto end;
 
 		/* Channel Table 0 */
 		value = 0;
@@ -926,13 +935,18 @@ void vnt_rf_table_download(struct vnt_private *priv)
 
 			memcpy(array, addr2, length);
 
-			vnt_control_out(priv, MESSAGE_TYPE_WRITE,
-					value, MESSAGE_REQUEST_RF_CH2,
-					length, array);
+			ret = vnt_control_out(priv, MESSAGE_TYPE_WRITE, value,
+					      MESSAGE_REQUEST_RF_CH2, length,
+					      array);
+			if (ret)
+				goto end;
 
 			length2 -= length;
 			value += length;
 			addr2 += length;
 		}
 	}
+
+end:
+	return ret;
 }
diff --git a/drivers/staging/vt6656/rf.h b/drivers/staging/vt6656/rf.h
index 6103117d6df5..7494546d71b8 100644
--- a/drivers/staging/vt6656/rf.h
+++ b/drivers/staging/vt6656/rf.h
@@ -44,6 +44,6 @@ int vnt_rf_write_embedded(struct vnt_private *priv, u32 data);
 int vnt_rf_setpower(struct vnt_private *priv, u32 rate, u32 channel);
 int vnt_rf_set_txpower(struct vnt_private *priv, u8 power, u32 rate);
 void vnt_rf_rssi_to_dbm(struct vnt_private *priv, u8 rssi, long *dbm);
-void vnt_rf_table_download(struct vnt_private *priv);
+int vnt_rf_table_download(struct vnt_private *priv);
 
 #endif /* __RF_H__ */
-- 
2.17.1

[PATCH 4/7] staging: vt6656: clean error path for firmware management

[Quentin Deslandes]

Avoid discarding return value of functions called during firmware
management process. Handle such return value and return 0 on success or
a negative errno value on error.

Signed-off-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk>
---
 drivers/staging/vt6656/firmware.c | 91 ++++++++++++++-----------------
 1 file changed, 40 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/vt6656/firmware.c b/drivers/staging/vt6656/firmware.c
index 38521c338917..60a00af250bf 100644
--- a/drivers/staging/vt6656/firmware.c
+++ b/drivers/staging/vt6656/firmware.c
@@ -30,98 +30,87 @@ int vnt_download_firmware(struct vnt_private *priv)
 {
 	struct device *dev = &priv->usb->dev;
 	const struct firmware *fw;
-	int status;
 	void *buffer = NULL;
-	bool result = false;
 	u16 length;
-	int ii, rc;
+	int ii;
+	int ret = 0;
 
 	dev_dbg(dev, "---->Download firmware\n");
 
-	rc = request_firmware(&fw, FIRMWARE_NAME, dev);
-	if (rc) {
+	ret = request_firmware(&fw, FIRMWARE_NAME, dev);
+	if (ret) {
 		dev_err(dev, "firmware file %s request failed (%d)\n",
-			FIRMWARE_NAME, rc);
-			goto out;
+			FIRMWARE_NAME, ret);
+		goto end;
 	}
 
 	buffer = kmalloc(FIRMWARE_CHUNK_SIZE, GFP_KERNEL);
-	if (!buffer)
+	if (!buffer) {
+		ret = -ENOMEM;
 		goto free_fw;
+	}
 
 	for (ii = 0; ii < fw->size; ii += FIRMWARE_CHUNK_SIZE) {
 		length = min_t(int, fw->size - ii, FIRMWARE_CHUNK_SIZE);
 		memcpy(buffer, fw->data + ii, length);
 
-		status = vnt_control_out(priv,
-					 0,
-					 0x1200 + ii,
-					 0x0000,
-					 length,
-					 buffer);
+		ret = vnt_control_out(priv, 0, 0x1200 + ii, 0x0000, length,
+				      buffer);
+		if (ret)
+			goto free_buffer;
 
 		dev_dbg(dev, "Download firmware...%d %zu\n", ii, fw->size);
-
-		if (status != STATUS_SUCCESS)
-			goto free_fw;
 	}
 
-	result = true;
+free_buffer:
+	kfree(buffer);
 free_fw:
 	release_firmware(fw);
-
-out:
-	kfree(buffer);
-
-	return result;
+end:
+	return ret;
 }
 MODULE_FIRMWARE(FIRMWARE_NAME);
 
 int vnt_firmware_branch_to_sram(struct vnt_private *priv)
 {
-	int status;
-
 	dev_dbg(&priv->usb->dev, "---->Branch to Sram\n");
 
-	status = vnt_control_out(priv,
-				 1,
-				 0x1200,
-				 0x0000,
-				 0,
-				 NULL);
-	return status == STATUS_SUCCESS;
+	return vnt_control_out(priv, 1, 0x1200, 0x0000, 0, NULL);
 }
 
 int vnt_check_firmware_version(struct vnt_private *priv)
 {
-	int status;
-
-	status = vnt_control_in(priv,
-				MESSAGE_TYPE_READ,
-				0,
-				MESSAGE_REQUEST_VERSION,
-				2,
-				(u8 *)&priv->firmware_version);
+	int ret = 0;
+
+	ret = vnt_control_in(priv, MESSAGE_TYPE_READ, 0,
+			     MESSAGE_REQUEST_VERSION, 2,
+			     (u8 *)&priv->firmware_version);
+	if (ret) {
+		dev_dbg(&priv->usb->dev,
+			"Could not get firmware version: %d.\n", ret);
+		goto end;
+	}
 
 	dev_dbg(&priv->usb->dev, "Firmware Version [%04x]\n",
 		priv->firmware_version);
 
-	if (status != STATUS_SUCCESS) {
-		dev_dbg(&priv->usb->dev, "Firmware Invalid.\n");
-		return false;
-	}
 	if (priv->firmware_version == 0xFFFF) {
 		dev_dbg(&priv->usb->dev, "In Loader.\n");
-		return false;
+		ret = -EINVAL;
+		goto end;
 	}
 
-	dev_dbg(&priv->usb->dev, "Firmware Version [%04x]\n",
-		priv->firmware_version);
-
 	if (priv->firmware_version < FIRMWARE_VERSION) {
 		/* branch to loader for download new firmware */
-		vnt_firmware_branch_to_sram(priv);
-		return false;
+		ret = vnt_firmware_branch_to_sram(priv);
+		if (ret) {
+			dev_dbg(&priv->usb->dev,
+				"Could not branch to SRAM: %d.\n", ret);
+		} else {
+			ret = -EINVAL;
+		}
 	}
-	return true;
+
+end:
+	return ret;
 }
-- 
2.17.1

[PATCH 6/7] staging: vt6656: clean-up registers initialization error path

[Quentin Deslandes]

Avoid discarding function's return code during register initialization.
Handle it instead and return 0 on success or a negative errno value on
error.

Signed-off-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk>
---
 drivers/staging/vt6656/main_usb.c | 163 ++++++++++++++++++------------
 1 file changed, 96 insertions(+), 67 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
index 5fd845cbdd52..8ed96e8eedbe 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -109,33 +109,38 @@ static void vnt_set_options(struct vnt_private *priv)
  */
 static int vnt_init_registers(struct vnt_private *priv)
 {
+	int ret = 0;
 	struct vnt_cmd_card_init *init_cmd = &priv->init_command;
 	struct vnt_rsp_card_init *init_rsp = &priv->init_response;
 	u8 antenna;
 	int ii;
-	int status = STATUS_SUCCESS;
 	u8 tmp;
 	u8 calib_tx_iq = 0, calib_tx_dc = 0, calib_rx_iq = 0;
 
 	dev_dbg(&priv->usb->dev, "---->INIbInitAdapter. [%d][%d]\n",
 		DEVICE_INIT_COLD, priv->packet_type);
 
-	if (!vnt_check_firmware_version(priv)) {
-		if (vnt_download_firmware(priv) == true) {
-			if (vnt_firmware_branch_to_sram(priv) == false) {
-				dev_dbg(&priv->usb->dev,
-					" vnt_firmware_branch_to_sram fail\n");
-				return false;
-			}
-		} else {
-			dev_dbg(&priv->usb->dev, "FIRMWAREbDownload fail\n");
-			return false;
+	ret = vnt_check_firmware_version(priv);
+	if (ret) {
+		ret = vnt_download_firmware(priv);
+		if (ret) {
+			dev_dbg(&priv->usb->dev,
+				"Could not download firmware: %d.\n", ret);
+			goto end;
+		}
+
+		ret = vnt_firmware_branch_to_sram(priv);
+		if (ret) {
+			dev_dbg(&priv->usb->dev,
+				"Could not branch to SRAM: %d.\n", ret);
+			goto end;
 		}
 	}
 
-	if (!vnt_vt3184_init(priv)) {
+	ret = vnt_vt3184_init(priv);
+	if (ret) {
 		dev_dbg(&priv->usb->dev, "vnt_vt3184_init fail\n");
-		return false;
+		goto end;
 	}
 
 	init_cmd->init_class = DEVICE_INIT_COLD;
@@ -146,28 +151,27 @@ static int vnt_init_registers(struct vnt_private *priv)
 	init_cmd->long_retry_limit = priv->long_retry_limit;
 
 	/* issue card_init command to device */
-	status = vnt_control_out(priv, MESSAGE_TYPE_CARDINIT, 0, 0,
-				 sizeof(struct vnt_cmd_card_init),
-				 (u8 *)init_cmd);
-	if (status != STATUS_SUCCESS) {
+	ret = vnt_control_out(priv, MESSAGE_TYPE_CARDINIT, 0, 0,
+			      sizeof(struct vnt_cmd_card_init),
+			      (u8 *)init_cmd);
+	if (ret) {
 		dev_dbg(&priv->usb->dev, "Issue Card init fail\n");
-		return false;
+		goto end;
 	}
 
-	status = vnt_control_in(priv, MESSAGE_TYPE_INIT_RSP, 0, 0,
-				sizeof(struct vnt_rsp_card_init),
-				(u8 *)init_rsp);
-	if (status != STATUS_SUCCESS) {
-		dev_dbg(&priv->usb->dev,
-			"Cardinit request in status fail!\n");
-		return false;
+	ret = vnt_control_in(priv, MESSAGE_TYPE_INIT_RSP, 0, 0,
+			     sizeof(struct vnt_rsp_card_init),
+			     (u8 *)init_rsp);
+	if (ret) {
+		dev_dbg(&priv->usb->dev, "Cardinit request in status fail!\n");
+		goto end;
 	}
 
 	/* local ID for AES functions */
-	status = vnt_control_in(priv, MESSAGE_TYPE_READ, MAC_REG_LOCALID,
-				MESSAGE_REQUEST_MACREG, 1, &priv->local_id);
-	if (status != STATUS_SUCCESS)
-		return false;
+	ret = vnt_control_in(priv, MESSAGE_TYPE_READ, MAC_REG_LOCALID,
+			     MESSAGE_REQUEST_MACREG, 1, &priv->local_id);
+	if (ret)
+		goto end;
 
 	/* do MACbSoftwareReset in MACvInitialize */
 
@@ -253,7 +257,9 @@ static int vnt_init_registers(struct vnt_private *priv)
 	}
 
 	/* Set initial antenna mode */
-	vnt_set_antenna_mode(priv, priv->rx_antenna_mode);
+	ret = vnt_set_antenna_mode(priv, priv->rx_antenna_mode);
+	if (ret)
+		goto end;
 
 	/* get Auto Fall Back type */
 	priv->auto_fb_ctrl = AUTO_FB_0;
@@ -275,33 +281,41 @@ static int vnt_init_registers(struct vnt_private *priv)
 				/* CR255, enable TX/RX IQ and
 				 * DC compensation mode
 				 */
-				vnt_control_out_u8(priv,
-						   MESSAGE_REQUEST_BBREG,
-						   0xff,
-						   0x03);
+				ret = vnt_control_out_u8(priv,
+							 MESSAGE_REQUEST_BBREG,
+							 0xff, 0x03);
+				if (ret)
+					goto end;
+
 				/* CR251, TX I/Q Imbalance Calibration */
-				vnt_control_out_u8(priv,
-						   MESSAGE_REQUEST_BBREG,
-						   0xfb,
-						   calib_tx_iq);
+				ret = vnt_control_out_u8(priv,
+							 MESSAGE_REQUEST_BBREG,
+							 0xfb, calib_tx_iq);
+				if (ret)
+					goto end;
+
 				/* CR252, TX DC-Offset Calibration */
-				vnt_control_out_u8(priv,
-						   MESSAGE_REQUEST_BBREG,
-						   0xfC,
-						   calib_tx_dc);
+				ret = vnt_control_out_u8(priv,
+							 MESSAGE_REQUEST_BBREG,
+							 0xfC, calib_tx_dc);
+				if (ret)
+					goto end;
+
 				/* CR253, RX I/Q Imbalance Calibration */
-				vnt_control_out_u8(priv,
-						   MESSAGE_REQUEST_BBREG,
-						   0xfd,
-						   calib_rx_iq);
+				ret = vnt_control_out_u8(priv,
+							 MESSAGE_REQUEST_BBREG,
+							 0xfd, calib_rx_iq);
+				if (ret)
+					goto end;
 			} else {
 				/* CR255, turn off
 				 * BB Calibration compensation
 				 */
-				vnt_control_out_u8(priv,
-						   MESSAGE_REQUEST_BBREG,
-						   0xff,
-						   0x0);
+				ret = vnt_control_out_u8(priv,
+							 MESSAGE_REQUEST_BBREG,
+							 0xff, 0x0);
+				if (ret)
+					goto end;
 			}
 		}
 	}
@@ -323,37 +337,52 @@ static int vnt_init_registers(struct vnt_private *priv)
 	else
 		priv->short_slot_time = false;
 
-	vnt_set_short_slot_time(priv);
+	ret = vnt_set_short_slot_time(priv);
+	if (ret)
+		goto end;
 
 	priv->radio_ctl = priv->eeprom[EEP_OFS_RADIOCTL];
 
 	if ((priv->radio_ctl & EEP_RADIOCTL_ENABLE) != 0) {
-		status = vnt_control_in(priv, MESSAGE_TYPE_READ,
-					MAC_REG_GPIOCTL1,
-					MESSAGE_REQUEST_MACREG, 1, &tmp);
+		ret = vnt_control_in(priv, MESSAGE_TYPE_READ,
+				     MAC_REG_GPIOCTL1, MESSAGE_REQUEST_MACREG,
+				     1, &tmp);
+		if (ret)
+			goto end;
 
-		if (status != STATUS_SUCCESS)
-			return false;
+		if ((tmp & GPIO3_DATA) == 0) {
+			ret = vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL1,
+						  GPIO3_INTMD);
+		} else {
+			ret = vnt_mac_reg_bits_off(priv, MAC_REG_GPIOCTL1,
+						   GPIO3_INTMD);
+		}
 
-		if ((tmp & GPIO3_DATA) == 0)
-			vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL1,
-					    GPIO3_INTMD);
-		else
-			vnt_mac_reg_bits_off(priv, MAC_REG_GPIOCTL1,
-					     GPIO3_INTMD);
+		if (ret)
+			goto end;
 	}
 
-	vnt_mac_set_led(priv, LEDSTS_TMLEN, 0x38);
 
-	vnt_mac_set_led(priv, LEDSTS_STS, LEDSTS_SLOW);
+	ret = vnt_mac_set_led(priv, LEDSTS_TMLEN, 0x38);
+	if (ret)
+		goto end;
 
-	vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL0, 0x01);
+	ret = vnt_mac_set_led(priv, LEDSTS_STS, LEDSTS_SLOW);
+	if (ret)
+		goto end;
 
-	vnt_radio_power_on(priv);
+	ret = vnt_mac_reg_bits_on(priv, MAC_REG_GPIOCTL0, 0x01);
+	if (ret)
+		goto end;
+
+	ret = vnt_radio_power_on(priv);
+	if (ret)
+		goto end;
 
 	dev_dbg(&priv->usb->dev, "<----INIbInitAdapter Exit\n");
 
-	return true;
+end:
+	return ret;
 }
 
 static void vnt_free_tx_bufs(struct vnt_private *priv)
-- 
2.17.1

Re: [PATCH 6/7] staging: vt6656: clean-up registers initialization error path

[Greg Kroah-Hartman]

On Mon, May 20, 2019 at 04:39:04PM +0000, Quentin Deslandes wrote:
> Avoid discarding function's return code during register initialization.
> Handle it instead and return 0 on success or a negative errno value on
> error.
> 
> Signed-off-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk>
> ---
>  drivers/staging/vt6656/main_usb.c | 163 ++++++++++++++++++------------
>  1 file changed, 96 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
> index 5fd845cbdd52..8ed96e8eedbe 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -109,33 +109,38 @@ static void vnt_set_options(struct vnt_private *priv)
>   */
>  static int vnt_init_registers(struct vnt_private *priv)
>  {
> +	int ret = 0;

Minor nit here, no need to set this to 0 as you instantly set it with
this call:

>  	struct vnt_cmd_card_init *init_cmd = &priv->init_command;
>  	struct vnt_rsp_card_init *init_rsp = &priv->init_response;
>  	u8 antenna;
>  	int ii;
> -	int status = STATUS_SUCCESS;
>  	u8 tmp;
>  	u8 calib_tx_iq = 0, calib_tx_dc = 0, calib_rx_iq = 0;
>  
>  	dev_dbg(&priv->usb->dev, "---->INIbInitAdapter. [%d][%d]\n",
>  		DEVICE_INIT_COLD, priv->packet_type);
>  
> -	if (!vnt_check_firmware_version(priv)) {
> -		if (vnt_download_firmware(priv) == true) {
> -			if (vnt_firmware_branch_to_sram(priv) == false) {
> -				dev_dbg(&priv->usb->dev,
> -					" vnt_firmware_branch_to_sram fail\n");
> -				return false;
> -			}
> -		} else {
> -			dev_dbg(&priv->usb->dev, "FIRMWAREbDownload fail\n");
> -			return false;
> +	ret = vnt_check_firmware_version(priv);

You can fix that up in a later patch :)

At first glance, these all look really good, thanks for doing this work.

greg k-h

[PATCH 5/7] staging: vt6656: use meaningful error code during buffer allocation

[Quentin Deslandes]

Check on called function's returned value for error and return 0 on
success or a negative errno value on error instead of a boolean value.

Signed-off-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk>
---
 drivers/staging/vt6656/main_usb.c | 42 ++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
index bfe952fe27bf..5fd845cbdd52 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -405,16 +405,19 @@ static void vnt_free_int_bufs(struct vnt_private *priv)
 	kfree(priv->int_buf.data_buf);
 }
 
-static bool vnt_alloc_bufs(struct vnt_private *priv)
+static int vnt_alloc_bufs(struct vnt_private *priv)
 {
+	int ret = 0;
 	struct vnt_usb_send_context *tx_context;
 	struct vnt_rcb *rcb;
 	int ii;
 
 	for (ii = 0; ii < priv->num_tx_context; ii++) {
 		tx_context = kmalloc(sizeof(*tx_context), GFP_KERNEL);
-		if (!tx_context)
+		if (!tx_context) {
+			ret = -ENOMEM;
 			goto free_tx;
+		}
 
 		priv->tx_context[ii] = tx_context;
 		tx_context->priv = priv;
@@ -422,16 +425,20 @@ static bool vnt_alloc_bufs(struct vnt_private *priv)
 
 		/* allocate URBs */
 		tx_context->urb = usb_alloc_urb(0, GFP_KERNEL);
-		if (!tx_context->urb)
+		if (!tx_context->urb) {
+			ret = -ENOMEM;
 			goto free_tx;
+		}
 
 		tx_context->in_use = false;
 	}
 
 	for (ii = 0; ii < priv->num_rcb; ii++) {
 		priv->rcb[ii] = kzalloc(sizeof(*priv->rcb[ii]), GFP_KERNEL);
-		if (!priv->rcb[ii])
+		if (!priv->rcb[ii]) {
+			ret = -ENOMEM;
 			goto free_rx_tx;
+		}
 
 		rcb = priv->rcb[ii];
 
@@ -439,39 +446,46 @@ static bool vnt_alloc_bufs(struct vnt_private *priv)
 
 		/* allocate URBs */
 		rcb->urb = usb_alloc_urb(0, GFP_KERNEL);
-		if (!rcb->urb)
+		if (!rcb->urb) {
+			ret = -ENOMEM;
 			goto free_rx_tx;
+		}
 
 		rcb->skb = dev_alloc_skb(priv->rx_buf_sz);
-		if (!rcb->skb)
+		if (!rcb->skb) {
+			ret = -ENOMEM;
 			goto free_rx_tx;
+		}
 
 		rcb->in_use = false;
 
 		/* submit rx urb */
-		if (vnt_submit_rx_urb(priv, rcb))
+		ret = vnt_submit_rx_urb(priv, rcb);
+		if (ret)
 			goto free_rx_tx;
 	}
 
 	priv->interrupt_urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!priv->interrupt_urb)
+	if (!priv->interrupt_urb) {
+		ret = -ENOMEM;
 		goto free_rx_tx;
+	}
 
 	priv->int_buf.data_buf = kmalloc(MAX_INTERRUPT_SIZE, GFP_KERNEL);
 	if (!priv->int_buf.data_buf) {
-		usb_free_urb(priv->interrupt_urb);
-		goto free_rx_tx;
+		ret = -ENOMEM;
+		goto free_rx_tx_urb;
 	}
 
-	return true;
+	return 0;
 
+free_rx_tx_urb:
+	usb_free_urb(priv->interrupt_urb);
 free_rx_tx:
 	vnt_free_rx_bufs(priv);
-
 free_tx:
 	vnt_free_tx_bufs(priv);
-
-	return false;
+	return ret;
 }
 
 static void vnt_tx_80211(struct ieee80211_hw *hw,
-- 
2.17.1

[PATCH 7/7] staging: vt6656: manage error path during device initialization

[Quentin Deslandes]

Check for error during device initialization callback and return a
meaningful error code or zero on success.

Signed-off-by: Quentin Deslandes <quentin.deslandes@itdev.co.uk>
---
 drivers/staging/vt6656/main_usb.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c b/drivers/staging/vt6656/main_usb.c
index 8ed96e8eedbe..856ba97aec4f 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -529,28 +529,34 @@ static void vnt_tx_80211(struct ieee80211_hw *hw,
 
 static int vnt_start(struct ieee80211_hw *hw)
 {
+	int ret = 0;
 	struct vnt_private *priv = hw->priv;
 
 	priv->rx_buf_sz = MAX_TOTAL_SIZE_WITH_ALL_HEADERS;
 
-	if (!vnt_alloc_bufs(priv)) {
+	ret = vnt_alloc_bufs(priv);
+	if (ret) {
 		dev_dbg(&priv->usb->dev, "vnt_alloc_bufs fail...\n");
-		return -ENOMEM;
+		goto err;
 	}
 
 	clear_bit(DEVICE_FLAGS_DISCONNECTED, &priv->flags);
 
-	if (vnt_init_registers(priv) == false) {
+	ret = vnt_init_registers(priv);
+	if (ret) {
 		dev_dbg(&priv->usb->dev, " init register fail\n");
 		goto free_all;
 	}
 
-	if (vnt_key_init_table(priv))
+	ret = vnt_key_init_table(priv);
+	if (ret)
 		goto free_all;
 
 	priv->int_interval = 1;  /* bInterval is set to 1 */
 
-	vnt_int_start_interrupt(priv);
+	ret = vnt_int_start_interrupt(priv);
+	if (ret)
+		goto free_all;
 
 	ieee80211_wake_queues(hw);
 
@@ -563,8 +569,8 @@ static int vnt_start(struct ieee80211_hw *hw)
 
 	usb_kill_urb(priv->interrupt_urb);
 	usb_free_urb(priv->interrupt_urb);
-
-	return -ENOMEM;
+err:
+	return ret;
 }
 
 static void vnt_stop(struct ieee80211_hw *hw)
-- 
2.17.1