diff mbox

[net-next,15/15] i40e: correctly setup ARQ descriptors

Message ID 1389087149-27962-16-git-send-email-jeffrey.t.kirsher@intel.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Kirsher, Jeffrey T Jan. 7, 2014, 9:32 a.m. UTC 
From: Mitch Williams <mitch.a.williams@intel.com>

When cleaning descriptors, we must set up ALL fields, not just the DMA
address. The initial setup does this correctly, but not the cleanup
code, so the firmware would process the ring exactly once and then fail.

Change-ID: I2930b83c76194b3016a8ac0fa693f9a573995640
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_adminq.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

David Miller Jan. 7, 2014, 8:34 p.m. UTC | #1 
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue,  7 Jan 2014 01:32:29 -0800

> +	memset((void *)desc, 0, sizeof(struct i40e_aq_desc));

This is terrible.

First of all, no pointer need ever be cast when passed into memset()
because it accepts void * which is cleanly implicitly cast from any
pointer type.  That's the whole reason memset accepts a void pointer,
so people don't need to ever cast the argument.

Secondly, using an explicit type to compute the size of the third
argument is error prone.

So please code this as the canonical:

	memset(desc, 0, sizeof(*desc));

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
index c87dc15..2b32084 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
@@ -933,6 +933,11 @@  i40e_status i40e_clean_arq_element(struct i40e_hw *hw,
 	 * size
 	 */
 	bi = &hw->aq.arq.r.arq_bi[ntc];
+	memset((void *)desc, 0, sizeof(struct i40e_aq_desc));
+
+	desc->flags = cpu_to_le16(I40E_AQ_FLAG_BUF);
+	if (hw->aq.arq_buf_size > I40E_AQ_LARGE_BUF)
+		desc->flags |= cpu_to_le16(I40E_AQ_FLAG_LB);
 	desc->datalen = cpu_to_le16((u16)bi->size);
 	desc->params.external.addr_high = cpu_to_le32(upper_32_bits(bi->pa));
 	desc->params.external.addr_low = cpu_to_le32(lower_32_bits(bi->pa));