linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] iwl3945: remove unused len_org variable
@ 2010-11-12  7:47 Stanislaw Gruszka
  2010-11-12  7:47 ` [PATCH 2/3] iwlagn: simplify iwlagn_tx_skb Stanislaw Gruszka
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Stanislaw Gruszka @ 2010-11-12  7:47 UTC (permalink / raw)
  To: Wey-Yi Guy, Intel Linux Wireless; +Cc: linux-wireless, Stanislaw Gruszka

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl3945-base.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 931c546..f2ce7dd 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -475,7 +475,7 @@ static int iwl3945_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
 	dma_addr_t phys_addr;
 	dma_addr_t txcmd_phys;
 	int txq_id = skb_get_queue_mapping(skb);
-	u16 len, idx, len_org, hdr_len; /* TODO: len_org is not used */
+	u16 len, idx, hdr_len;
 	u8 id;
 	u8 unicast;
 	u8 sta_id;
@@ -612,15 +612,8 @@ static int iwl3945_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
 	 */
 	len = sizeof(struct iwl3945_tx_cmd) +
 			sizeof(struct iwl_cmd_header) + hdr_len;
-
-	len_org = len;
 	len = (len + 3) & ~3;
 
-	if (len_org != len)
-		len_org = 1;
-	else
-		len_org = 0;
-
 	/* Physical address of this Tx command's header (not MAC header!),
 	 * within command buffer array. */
 	txcmd_phys = pci_map_single(priv->pci_dev, &out_cmd->hdr,
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] iwlagn: simplify iwlagn_tx_skb
  2010-11-12  7:47 [PATCH 1/3] iwl3945: remove unused len_org variable Stanislaw Gruszka
@ 2010-11-12  7:47 ` Stanislaw Gruszka
  2010-11-12 15:52   ` Guy, Wey-Yi
  2010-11-12  7:47 ` [PATCH 3/3] iwlwifi: kill elapsed_jiffies Stanislaw Gruszka
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Stanislaw Gruszka @ 2010-11-12  7:47 UTC (permalink / raw)
  To: Wey-Yi Guy, Intel Linux Wireless; +Cc: linux-wireless, Stanislaw Gruszka

We can simplify length calculation in iwlagn_tx_skb, that function
is enough complex, without fuzz it more than necessary.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn-tx.c |   33 ++++++++++------------------
 1 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
index 2b078a9..1205cec 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
@@ -522,7 +522,7 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
 	dma_addr_t phys_addr;
 	dma_addr_t txcmd_phys;
 	dma_addr_t scratch_phys;
-	u16 len, len_org, firstlen, secondlen;
+	u16 len, firstlen, secondlen;
 	u16 seq_number = 0;
 	__le16 fc;
 	u8 hdr_len;
@@ -687,30 +687,23 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
 	 */
 	len = sizeof(struct iwl_tx_cmd) +
 		sizeof(struct iwl_cmd_header) + hdr_len;
-
-	len_org = len;
-	firstlen = len = (len + 3) & ~3;
-
-	if (len_org != len)
-		len_org = 1;
-	else
-		len_org = 0;
+	firstlen = (len + 3) & ~3;
 
 	/* Tell NIC about any 2-byte padding after MAC header */
-	if (len_org)
+	if (firstlen != len)
 		tx_cmd->tx_flags |= TX_CMD_FLG_MH_PAD_MSK;
 
 	/* Physical address of this Tx command's header (not MAC header!),
 	 * within command buffer array. */
 	txcmd_phys = pci_map_single(priv->pci_dev,
-				    &out_cmd->hdr, len,
+				    &out_cmd->hdr, firstlen,
 				    PCI_DMA_BIDIRECTIONAL);
 	dma_unmap_addr_set(out_meta, mapping, txcmd_phys);
-	dma_unmap_len_set(out_meta, len, len);
+	dma_unmap_len_set(out_meta, len, firstlen);
 	/* Add buffer containing Tx command and MAC(!) header to TFD's
 	 * first entry */
 	priv->cfg->ops->lib->txq_attach_buf_to_tfd(priv, txq,
-						   txcmd_phys, len, 1, 0);
+						   txcmd_phys, firstlen, 1, 0);
 
 	if (!ieee80211_has_morefrags(hdr->frame_control)) {
 		txq->need_update = 1;
@@ -721,23 +714,21 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
 
 	/* Set up TFD's 2nd entry to point directly to remainder of skb,
 	 * if any (802.11 null frames have no payload). */
-	secondlen = len = skb->len - hdr_len;
-	if (len) {
+	secondlen = skb->len - hdr_len;
+	if (secondlen > 0) {
 		phys_addr = pci_map_single(priv->pci_dev, skb->data + hdr_len,
-					   len, PCI_DMA_TODEVICE);
+					   secondlen, PCI_DMA_TODEVICE);
 		priv->cfg->ops->lib->txq_attach_buf_to_tfd(priv, txq,
-							   phys_addr, len,
+							   phys_addr, secondlen,
 							   0, 0);
 	}
 
 	scratch_phys = txcmd_phys + sizeof(struct iwl_cmd_header) +
 				offsetof(struct iwl_tx_cmd, scratch);
 
-	len = sizeof(struct iwl_tx_cmd) +
-		sizeof(struct iwl_cmd_header) + hdr_len;
 	/* take back ownership of DMA buffer to enable update */
 	pci_dma_sync_single_for_cpu(priv->pci_dev, txcmd_phys,
-				    len, PCI_DMA_BIDIRECTIONAL);
+				    firstlen, PCI_DMA_BIDIRECTIONAL);
 	tx_cmd->dram_lsb_ptr = cpu_to_le32(scratch_phys);
 	tx_cmd->dram_msb_ptr = iwl_get_dma_hi_addr(scratch_phys);
 
@@ -753,7 +744,7 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
 						     le16_to_cpu(tx_cmd->len));
 
 	pci_dma_sync_single_for_device(priv->pci_dev, txcmd_phys,
-				       len, PCI_DMA_BIDIRECTIONAL);
+				       firstlen, PCI_DMA_BIDIRECTIONAL);
 
 	trace_iwlwifi_dev_tx(priv,
 			     &((struct iwl_tfd *)txq->tfds)[txq->q.write_ptr],
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] iwlwifi: kill elapsed_jiffies
  2010-11-12  7:47 [PATCH 1/3] iwl3945: remove unused len_org variable Stanislaw Gruszka
  2010-11-12  7:47 ` [PATCH 2/3] iwlagn: simplify iwlagn_tx_skb Stanislaw Gruszka
@ 2010-11-12  7:47 ` Stanislaw Gruszka
  2010-11-12 15:53   ` Guy, Wey-Yi
  2010-11-12  8:19 ` [PATCH 1/3] iwl3945: remove unused len_org variable Sedat Dilek
  2010-11-12 15:32 ` Guy, Wey-Yi
  3 siblings, 1 reply; 10+ messages in thread
From: Stanislaw Gruszka @ 2010-11-12  7:47 UTC (permalink / raw)
  To: Wey-Yi Guy, Intel Linux Wireless; +Cc: linux-wireless, Stanislaw Gruszka

Subtract of jiffies is fine even if one variable overwrap.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-helpers.h |    9 ---------
 drivers/net/wireless/iwlwifi/iwl-scan.c    |    3 +--
 2 files changed, 1 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-helpers.h b/drivers/net/wireless/iwlwifi/iwl-helpers.h
index 1aaef70..9233683 100644
--- a/drivers/net/wireless/iwlwifi/iwl-helpers.h
+++ b/drivers/net/wireless/iwlwifi/iwl-helpers.h
@@ -44,15 +44,6 @@ static inline struct ieee80211_conf *ieee80211_get_hw_conf(
 	return &hw->conf;
 }
 
-static inline unsigned long elapsed_jiffies(unsigned long start,
-					    unsigned long end)
-{
-	if (end >= start)
-		return end - start;
-
-	return end + (MAX_JIFFY_OFFSET - start) + 1;
-}
-
 /**
  * iwl_queue_inc_wrap - increment queue index, wrap back to beginning
  * @index -- current index
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index e1aa0e1..12d9363 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -252,8 +252,7 @@ static void iwl_rx_scan_complete_notif(struct iwl_priv *priv,
 
 	IWL_DEBUG_SCAN(priv, "Scan on %sGHz took %dms\n",
 		       (priv->scan_band == IEEE80211_BAND_2GHZ) ? "2.4" : "5.2",
-		       jiffies_to_msecs(elapsed_jiffies
-					(priv->scan_start, jiffies)));
+		       jiffies_to_msecs(jiffies - priv->scan_start));
 
 	queue_work(priv->workqueue, &priv->scan_completed);
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] iwl3945: remove unused len_org variable
  2010-11-12  7:47 [PATCH 1/3] iwl3945: remove unused len_org variable Stanislaw Gruszka
  2010-11-12  7:47 ` [PATCH 2/3] iwlagn: simplify iwlagn_tx_skb Stanislaw Gruszka
  2010-11-12  7:47 ` [PATCH 3/3] iwlwifi: kill elapsed_jiffies Stanislaw Gruszka
@ 2010-11-12  8:19 ` Sedat Dilek
  2010-11-12  8:56   ` Stanislaw Gruszka
  2010-11-12 15:32 ` Guy, Wey-Yi
  3 siblings, 1 reply; 10+ messages in thread
From: Sedat Dilek @ 2010-11-12  8:19 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Wey-Yi Guy, Intel Linux Wireless, linux-wireless

Against which GIT tree has your patchset to be applied?

- Sedat -

On Fri, Nov 12, 2010 at 8:47 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |    9 +--------
>  1 files changed, 1 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index 931c546..f2ce7dd 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -475,7 +475,7 @@ static int iwl3945_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
>        dma_addr_t phys_addr;
>        dma_addr_t txcmd_phys;
>        int txq_id = skb_get_queue_mapping(skb);
> -       u16 len, idx, len_org, hdr_len; /* TODO: len_org is not used */
> +       u16 len, idx, hdr_len;
>        u8 id;
>        u8 unicast;
>        u8 sta_id;
> @@ -612,15 +612,8 @@ static int iwl3945_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
>         */
>        len = sizeof(struct iwl3945_tx_cmd) +
>                        sizeof(struct iwl_cmd_header) + hdr_len;
> -
> -       len_org = len;
>        len = (len + 3) & ~3;
>
> -       if (len_org != len)
> -               len_org = 1;
> -       else
> -               len_org = 0;
> -
>        /* Physical address of this Tx command's header (not MAC header!),
>         * within command buffer array. */
>        txcmd_phys = pci_map_single(priv->pci_dev, &out_cmd->hdr,
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] iwl3945: remove unused len_org variable
  2010-11-12  8:19 ` [PATCH 1/3] iwl3945: remove unused len_org variable Sedat Dilek
@ 2010-11-12  8:56   ` Stanislaw Gruszka
  0 siblings, 0 replies; 10+ messages in thread
From: Stanislaw Gruszka @ 2010-11-12  8:56 UTC (permalink / raw)
  To: sedat.dilek; +Cc: Wey-Yi Guy, Intel Linux Wireless, linux-wireless

On Fri, Nov 12, 2010 at 09:19:42AM +0100, Sedat Dilek wrote:
> Against which GIT tree has your patchset to be applied?

wireless-testing

Stanislaw

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] iwl3945: remove unused len_org variable
  2010-11-12  7:47 [PATCH 1/3] iwl3945: remove unused len_org variable Stanislaw Gruszka
                   ` (2 preceding siblings ...)
  2010-11-12  8:19 ` [PATCH 1/3] iwl3945: remove unused len_org variable Sedat Dilek
@ 2010-11-12 15:32 ` Guy, Wey-Yi
  3 siblings, 0 replies; 10+ messages in thread
From: Guy, Wey-Yi @ 2010-11-12 15:32 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless@vger.kernel.org

On Thu, 2010-11-11 at 23:47 -0800, Stanislaw Gruszka wrote:
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] iwlagn: simplify iwlagn_tx_skb
  2010-11-12  7:47 ` [PATCH 2/3] iwlagn: simplify iwlagn_tx_skb Stanislaw Gruszka
@ 2010-11-12 15:52   ` Guy, Wey-Yi
  2010-11-12 16:46     ` Stanislaw Gruszka
  0 siblings, 1 reply; 10+ messages in thread
From: Guy, Wey-Yi @ 2010-11-12 15:52 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless@vger.kernel.org

Hi Gruszka,

On Thu, 2010-11-11 at 23:47 -0800, Stanislaw Gruszka wrote:
> We can simplify length calculation in iwlagn_tx_skb, that function
> is enough complex, without fuzz it more than necessary.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-agn-tx.c |   33 ++++++++++------------------
>  1 files changed, 12 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
> index 2b078a9..1205cec 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
> @@ -522,7 +522,7 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
>  	dma_addr_t phys_addr;
>  	dma_addr_t txcmd_phys;
>  	dma_addr_t scratch_phys;
> -	u16 len, len_org, firstlen, secondlen;
> +	u16 len, firstlen, secondlen;
>  	u16 seq_number = 0;
>  	__le16 fc;
>  	u8 hdr_len;
> @@ -687,30 +687,23 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
>  	 */
>  	len = sizeof(struct iwl_tx_cmd) +
>  		sizeof(struct iwl_cmd_header) + hdr_len;
> -
> -	len_org = len;
> -	firstlen = len = (len + 3) & ~3;
> -
> -	if (len_org != len)
> -		len_org = 1;
> -	else
> -		len_org = 0;
> +	firstlen = (len + 3) & ~3;
>  
>  	/* Tell NIC about any 2-byte padding after MAC header */
> -	if (len_org)
> +	if (firstlen != len)
>  		tx_cmd->tx_flags |= TX_CMD_FLG_MH_PAD_MSK;
>  
>  	/* Physical address of this Tx command's header (not MAC header!),
>  	 * within command buffer array. */
>  	txcmd_phys = pci_map_single(priv->pci_dev,
> -				    &out_cmd->hdr, len,
> +				    &out_cmd->hdr, firstlen,
>  				    PCI_DMA_BIDIRECTIONAL);
>  	dma_unmap_addr_set(out_meta, mapping, txcmd_phys);
> -	dma_unmap_len_set(out_meta, len, len);
> +	dma_unmap_len_set(out_meta, len, firstlen);

it is possible len != firstlen, right?

>  	/* Add buffer containing Tx command and MAC(!) header to TFD's
>  	 * first entry */
>  	priv->cfg->ops->lib->txq_attach_buf_to_tfd(priv, txq,
> -						   txcmd_phys, len, 1, 0);
> +						   txcmd_phys, firstlen, 1, 0);
>  
>  	if (!ieee80211_has_morefrags(hdr->frame_control)) {
>  		txq->need_update = 1;
> @@ -721,23 +714,21 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
>  
>  	/* Set up TFD's 2nd entry to point directly to remainder of skb,
>  	 * if any (802.11 null frames have no payload). */
> -	secondlen = len = skb->len - hdr_len;
> -	if (len) {
> +	secondlen = skb->len - hdr_len;
> +	if (secondlen > 0) {
>  		phys_addr = pci_map_single(priv->pci_dev, skb->data + hdr_len,
> -					   len, PCI_DMA_TODEVICE);
> +					   secondlen, PCI_DMA_TODEVICE);
>  		priv->cfg->ops->lib->txq_attach_buf_to_tfd(priv, txq,
> -							   phys_addr, len,
> +							   phys_addr, secondlen,
>  							   0, 0);
>  	}
>  
>  	scratch_phys = txcmd_phys + sizeof(struct iwl_cmd_header) +
>  				offsetof(struct iwl_tx_cmd, scratch);
>  
> -	len = sizeof(struct iwl_tx_cmd) +
> -		sizeof(struct iwl_cmd_header) + hdr_len;
>  	/* take back ownership of DMA buffer to enable update */
>  	pci_dma_sync_single_for_cpu(priv->pci_dev, txcmd_phys,
> -				    len, PCI_DMA_BIDIRECTIONAL);
> +				    firstlen, PCI_DMA_BIDIRECTIONAL);

same here, "firstlen" not necessary equal "len"

>  	tx_cmd->dram_lsb_ptr = cpu_to_le32(scratch_phys);
>  	tx_cmd->dram_msb_ptr = iwl_get_dma_hi_addr(scratch_phys);
>  
> @@ -753,7 +744,7 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
>  						     le16_to_cpu(tx_cmd->len));
>  
>  	pci_dma_sync_single_for_device(priv->pci_dev, txcmd_phys,
> -				       len, PCI_DMA_BIDIRECTIONAL);
> +				       firstlen, PCI_DMA_BIDIRECTIONAL);

same here

>  
>  	trace_iwlwifi_dev_tx(priv,
>  			     &((struct iwl_tfd *)txq->tfds)[txq->q.write_ptr],

Wey


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] iwlwifi: kill elapsed_jiffies
  2010-11-12  7:47 ` [PATCH 3/3] iwlwifi: kill elapsed_jiffies Stanislaw Gruszka
@ 2010-11-12 15:53   ` Guy, Wey-Yi
  0 siblings, 0 replies; 10+ messages in thread
From: Guy, Wey-Yi @ 2010-11-12 15:53 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless@vger.kernel.org

On Thu, 2010-11-11 at 23:47 -0800, Stanislaw Gruszka wrote:
> Subtract of jiffies is fine even if one variable overwrap.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] iwlagn: simplify iwlagn_tx_skb
  2010-11-12 15:52   ` Guy, Wey-Yi
@ 2010-11-12 16:46     ` Stanislaw Gruszka
  2010-11-12 17:51       ` Guy, Wey-Yi
  0 siblings, 1 reply; 10+ messages in thread
From: Stanislaw Gruszka @ 2010-11-12 16:46 UTC (permalink / raw)
  To: Guy, Wey-Yi; +Cc: Intel Linux Wireless, linux-wireless@vger.kernel.org

Hi,

On Fri, Nov 12, 2010 at 07:52:05AM -0800, Guy, Wey-Yi wrote:
> On Thu, 2010-11-11 at 23:47 -0800, Stanislaw Gruszka wrote:
> > We can simplify length calculation in iwlagn_tx_skb, that function
> > is enough complex, without fuzz it more than necessary.
> > 
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > ---
> >  drivers/net/wireless/iwlwifi/iwl-agn-tx.c |   33 ++++++++++------------------
> >  1 files changed, 12 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
> > index 2b078a9..1205cec 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
> > @@ -522,7 +522,7 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
> >  	dma_addr_t phys_addr;
> >  	dma_addr_t txcmd_phys;
> >  	dma_addr_t scratch_phys;
> > -	u16 len, len_org, firstlen, secondlen;
> > +	u16 len, firstlen, secondlen;
> >  	u16 seq_number = 0;
> >  	__le16 fc;
> >  	u8 hdr_len;
> > @@ -687,30 +687,23 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
> >  	 */
> >  	len = sizeof(struct iwl_tx_cmd) +
> >  		sizeof(struct iwl_cmd_header) + hdr_len;
> > -
> > -	len_org = len;
> > -	firstlen = len = (len + 3) & ~3;
> > -
> > -	if (len_org != len)
> > -		len_org = 1;
> > -	else
> > -		len_org = 0;
> > +	firstlen = (len + 3) & ~3;
> >  
> >  	/* Tell NIC about any 2-byte padding after MAC header */
> > -	if (len_org)
> > +	if (firstlen != len)
> >  		tx_cmd->tx_flags |= TX_CMD_FLG_MH_PAD_MSK;
> >  
> >  	/* Physical address of this Tx command's header (not MAC header!),
> >  	 * within command buffer array. */
> >  	txcmd_phys = pci_map_single(priv->pci_dev,
> > -				    &out_cmd->hdr, len,
> > +				    &out_cmd->hdr, firstlen,
> >  				    PCI_DMA_BIDIRECTIONAL);
> >  	dma_unmap_addr_set(out_meta, mapping, txcmd_phys);
> > -	dma_unmap_len_set(out_meta, len, len);
> > +	dma_unmap_len_set(out_meta, len, firstlen);
> 
> it is possible len != firstlen, right?

Yes it is, firstlen is aligned up to 4 byes value, txcmd_phys dma map
size is firstlen and in consequence I use it every time with txcmd_phys.

> >  	/* Add buffer containing Tx command and MAC(!) header to TFD's
> >  	 * first entry */
> >  	priv->cfg->ops->lib->txq_attach_buf_to_tfd(priv, txq,
> > -						   txcmd_phys, len, 1, 0);
> > +						   txcmd_phys, firstlen, 1, 0);
> >  
> >  	if (!ieee80211_has_morefrags(hdr->frame_control)) {
> >  		txq->need_update = 1;
> > @@ -721,23 +714,21 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
> >  
> >  	/* Set up TFD's 2nd entry to point directly to remainder of skb,
> >  	 * if any (802.11 null frames have no payload). */
> > -	secondlen = len = skb->len - hdr_len;
> > -	if (len) {
> > +	secondlen = skb->len - hdr_len;
> > +	if (secondlen > 0) {
> >  		phys_addr = pci_map_single(priv->pci_dev, skb->data + hdr_len,
> > -					   len, PCI_DMA_TODEVICE);
> > +					   secondlen, PCI_DMA_TODEVICE);
> >  		priv->cfg->ops->lib->txq_attach_buf_to_tfd(priv, txq,
> > -							   phys_addr, len,
> > +							   phys_addr, secondlen,
> >  							   0, 0);
> >  	}
> >  
> >  	scratch_phys = txcmd_phys + sizeof(struct iwl_cmd_header) +
> >  				offsetof(struct iwl_tx_cmd, scratch);
> >  
> > -	len = sizeof(struct iwl_tx_cmd) +
> > -		sizeof(struct iwl_cmd_header) + hdr_len;
> >  	/* take back ownership of DMA buffer to enable update */
> >  	pci_dma_sync_single_for_cpu(priv->pci_dev, txcmd_phys,
> > -				    len, PCI_DMA_BIDIRECTIONAL);
> > +				    firstlen, PCI_DMA_BIDIRECTIONAL);
> 
> same here, "firstlen" not necessary equal "len"

I guess its enough to sync just "len" but I prefer to use the
size we used to map dma.

Stanislaw

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] iwlagn: simplify iwlagn_tx_skb
  2010-11-12 16:46     ` Stanislaw Gruszka
@ 2010-11-12 17:51       ` Guy, Wey-Yi
  0 siblings, 0 replies; 10+ messages in thread
From: Guy, Wey-Yi @ 2010-11-12 17:51 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Intel Linux Wireless, linux-wireless@vger.kernel.org

On Fri, 2010-11-12 at 08:46 -0800, Stanislaw Gruszka wrote:
> Hi,
> 
> On Fri, Nov 12, 2010 at 07:52:05AM -0800, Guy, Wey-Yi wrote:
> > On Thu, 2010-11-11 at 23:47 -0800, Stanislaw Gruszka wrote:
> > > We can simplify length calculation in iwlagn_tx_skb, that function
> > > is enough complex, without fuzz it more than necessary.
> > > 
> > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > > ---
> > >  drivers/net/wireless/iwlwifi/iwl-agn-tx.c |   33 ++++++++++------------------
> > >  1 files changed, 12 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
> > > index 2b078a9..1205cec 100644
> > > --- a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
> > > +++ b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c
> > > @@ -522,7 +522,7 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
> > >  	dma_addr_t phys_addr;
> > >  	dma_addr_t txcmd_phys;
> > >  	dma_addr_t scratch_phys;
> > > -	u16 len, len_org, firstlen, secondlen;
> > > +	u16 len, firstlen, secondlen;
> > >  	u16 seq_number = 0;
> > >  	__le16 fc;
> > >  	u8 hdr_len;
> > > @@ -687,30 +687,23 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
> > >  	 */
> > >  	len = sizeof(struct iwl_tx_cmd) +
> > >  		sizeof(struct iwl_cmd_header) + hdr_len;
> > > -
> > > -	len_org = len;
> > > -	firstlen = len = (len + 3) & ~3;
> > > -
> > > -	if (len_org != len)
> > > -		len_org = 1;
> > > -	else
> > > -		len_org = 0;
> > > +	firstlen = (len + 3) & ~3;
> > >  
> > >  	/* Tell NIC about any 2-byte padding after MAC header */
> > > -	if (len_org)
> > > +	if (firstlen != len)
> > >  		tx_cmd->tx_flags |= TX_CMD_FLG_MH_PAD_MSK;
> > >  
> > >  	/* Physical address of this Tx command's header (not MAC header!),
> > >  	 * within command buffer array. */
> > >  	txcmd_phys = pci_map_single(priv->pci_dev,
> > > -				    &out_cmd->hdr, len,
> > > +				    &out_cmd->hdr, firstlen,
> > >  				    PCI_DMA_BIDIRECTIONAL);
> > >  	dma_unmap_addr_set(out_meta, mapping, txcmd_phys);
> > > -	dma_unmap_len_set(out_meta, len, len);
> > > +	dma_unmap_len_set(out_meta, len, firstlen);
> > 
> > it is possible len != firstlen, right?
> 
> Yes it is, firstlen is aligned up to 4 byes value, txcmd_phys dma map
> size is firstlen and in consequence I use it every time with txcmd_phys.
> 
> > >  	/* Add buffer containing Tx command and MAC(!) header to TFD's
> > >  	 * first entry */
> > >  	priv->cfg->ops->lib->txq_attach_buf_to_tfd(priv, txq,
> > > -						   txcmd_phys, len, 1, 0);
> > > +						   txcmd_phys, firstlen, 1, 0);
> > >  
> > >  	if (!ieee80211_has_morefrags(hdr->frame_control)) {
> > >  		txq->need_update = 1;
> > > @@ -721,23 +714,21 @@ int iwlagn_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
> > >  
> > >  	/* Set up TFD's 2nd entry to point directly to remainder of skb,
> > >  	 * if any (802.11 null frames have no payload). */
> > > -	secondlen = len = skb->len - hdr_len;
> > > -	if (len) {
> > > +	secondlen = skb->len - hdr_len;
> > > +	if (secondlen > 0) {
> > >  		phys_addr = pci_map_single(priv->pci_dev, skb->data + hdr_len,
> > > -					   len, PCI_DMA_TODEVICE);
> > > +					   secondlen, PCI_DMA_TODEVICE);
> > >  		priv->cfg->ops->lib->txq_attach_buf_to_tfd(priv, txq,
> > > -							   phys_addr, len,
> > > +							   phys_addr, secondlen,
> > >  							   0, 0);
> > >  	}
> > >  
> > >  	scratch_phys = txcmd_phys + sizeof(struct iwl_cmd_header) +
> > >  				offsetof(struct iwl_tx_cmd, scratch);
> > >  
> > > -	len = sizeof(struct iwl_tx_cmd) +
> > > -		sizeof(struct iwl_cmd_header) + hdr_len;
> > >  	/* take back ownership of DMA buffer to enable update */
> > >  	pci_dma_sync_single_for_cpu(priv->pci_dev, txcmd_phys,
> > > -				    len, PCI_DMA_BIDIRECTIONAL);
> > > +				    firstlen, PCI_DMA_BIDIRECTIONAL);
> > 
> > same here, "firstlen" not necessary equal "len"
> 
> I guess its enough to sync just "len" but I prefer to use the
> size we used to map dma.
> 
ok, it make sense

Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>




^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-11-12 17:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-12  7:47 [PATCH 1/3] iwl3945: remove unused len_org variable Stanislaw Gruszka
2010-11-12  7:47 ` [PATCH 2/3] iwlagn: simplify iwlagn_tx_skb Stanislaw Gruszka
2010-11-12 15:52   ` Guy, Wey-Yi
2010-11-12 16:46     ` Stanislaw Gruszka
2010-11-12 17:51       ` Guy, Wey-Yi
2010-11-12  7:47 ` [PATCH 3/3] iwlwifi: kill elapsed_jiffies Stanislaw Gruszka
2010-11-12 15:53   ` Guy, Wey-Yi
2010-11-12  8:19 ` [PATCH 1/3] iwl3945: remove unused len_org variable Sedat Dilek
2010-11-12  8:56   ` Stanislaw Gruszka
2010-11-12 15:32 ` Guy, Wey-Yi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).