public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] staging: et131x: Use skb_headlen() where appropriate
@ 2012-10-17 21:15 Mark Einon
  2012-10-17 21:15 ` [PATCH 2/2] staging: et131x: Fix 64bit tx dma address handling Mark Einon
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Einon @ 2012-10-17 21:15 UTC (permalink / raw)
  To: devel, gregkh; +Cc: linux-kernel, Mark Einon

(skb->len - skb->data_len) is used in several places in the et131x
driver code. Converted all instances of this to use skb_headlen()
which is more readable.

Signed-off-by: Mark Einon <mark.einon@gmail.com>
---
 drivers/staging/et131x/et131x.c |   25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index 029725c..f0fe4bd 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -3313,12 +3313,11 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
 			 * This will work until we determine why the hardware
 			 * doesn't seem to like large fragments.
 			 */
-			if ((skb->len - skb->data_len) <= 1514) {
+			if (skb_headlen(skb) <= 1514) {
 				desc[frag].addr_hi = 0;
 				/* Low 16bits are length, high is vlan and
 				   unused currently so zero */
-				desc[frag].len_vlan =
-					skb->len - skb->data_len;
+				desc[frag].len_vlan = skb_headlen(skb);
 
 				/* NOTE: Here, the dma_addr_t returned from
 				 * dma_map_single() is implicitly cast as a
@@ -3331,13 +3330,11 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
 				desc[frag++].addr_lo =
 				    dma_map_single(&adapter->pdev->dev,
 						   skb->data,
-						   skb->len -
-						   skb->data_len,
+						   skb_headlen(skb),
 						   DMA_TO_DEVICE);
 			} else {
 				desc[frag].addr_hi = 0;
-				desc[frag].len_vlan =
-				    (skb->len - skb->data_len) / 2;
+				desc[frag].len_vlan = skb_headlen(skb) / 2;
 
 				/* NOTE: Here, the dma_addr_t returned from
 				 * dma_map_single() is implicitly cast as a
@@ -3350,13 +3347,11 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
 				desc[frag++].addr_lo =
 				    dma_map_single(&adapter->pdev->dev,
 						   skb->data,
-						   ((skb->len -
-						     skb->data_len) / 2),
+						   (skb_headlen(skb) / 2),
 						   DMA_TO_DEVICE);
 				desc[frag].addr_hi = 0;
 
-				desc[frag].len_vlan =
-				    (skb->len - skb->data_len) / 2;
+				desc[frag].len_vlan = skb_headlen(skb) / 2;
 
 				/* NOTE: Here, the dma_addr_t returned from
 				 * dma_map_single() is implicitly cast as a
@@ -3369,10 +3364,8 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
 				desc[frag++].addr_lo =
 				    dma_map_single(&adapter->pdev->dev,
 						   skb->data +
-						   ((skb->len -
-						     skb->data_len) / 2),
-						   ((skb->len -
-						     skb->data_len) / 2),
+							 (skb_headlen(skb) / 2),
+						   (skb_headlen(skb) / 2),
 						   DMA_TO_DEVICE);
 			}
 		} else {
@@ -3521,7 +3514,7 @@ static int send_packet(struct sk_buff *skb, struct et131x_adapter *adapter)
 
 	tcb->skb = skb;
 
-	if (skb->data != NULL && skb->len - skb->data_len >= 6) {
+	if (skb->data != NULL && skb_headlen(skb) >= 6) {
 		shbufva = (u16 *) skb->data;
 
 		if ((shbufva[0] == 0xffff) &&
-- 
1.7.9.5


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

* [PATCH 2/2] staging: et131x: Fix 64bit tx dma address handling
  2012-10-17 21:15 [PATCH 1/2] staging: et131x: Use skb_headlen() where appropriate Mark Einon
@ 2012-10-17 21:15 ` Mark Einon
  2012-10-18  0:31   ` Jeffrey Ladouceur
  2012-10-18 20:34   ` [PATCH 2/2 v2] " Mark Einon
  0 siblings, 2 replies; 6+ messages in thread
From: Mark Einon @ 2012-10-17 21:15 UTC (permalink / raw)
  To: devel, gregkh; +Cc: linux-kernel, Mark Einon

The driver checks that the device can handle 64bit DMA addressing in
et131x_pci_setup(), but then assumes that the top dword of a tx dma
address is always zero when creating a dma mapping in nic_send_packet().
Fix the mapping to use the higher dword of the dma_addr_t returned by
dma_map_single() and skb_frag_dma_map().

Also remove incorrect comments stating that dma_map_single() only returns
a 32 bit address.

Signed-off-by: Mark Einon <mark.einon@gmail.com>
---
 drivers/staging/et131x/et131x.c |  102 +++++++++++++++------------------------
 1 file changed, 39 insertions(+), 63 deletions(-)

diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index f0fe4bd..83643be 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -3285,6 +3285,7 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
 	struct skb_frag_struct *frags = &skb_shinfo(skb)->frags[0];
 	unsigned long flags;
 	struct phy_device *phydev = adapter->phydev;
+	dma_addr_t dma_addr;
 
 	/* Part of the optimizations of this send routine restrict us to
 	 * sending 24 fragments at a pass.  In practice we should never see
@@ -3314,77 +3315,46 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
 			 * doesn't seem to like large fragments.
 			 */
 			if (skb_headlen(skb) <= 1514) {
-				desc[frag].addr_hi = 0;
 				/* Low 16bits are length, high is vlan and
 				   unused currently so zero */
 				desc[frag].len_vlan = skb_headlen(skb);
-
-				/* NOTE: Here, the dma_addr_t returned from
-				 * dma_map_single() is implicitly cast as a
-				 * u32. Although dma_addr_t can be
-				 * 64-bit, the address returned by
-				 * dma_map_single() is always 32-bit
-				 * addressable (as defined by the pci/dma
-				 * subsystem)
-				 */
-				desc[frag++].addr_lo =
-				    dma_map_single(&adapter->pdev->dev,
-						   skb->data,
-						   skb_headlen(skb),
-						   DMA_TO_DEVICE);
+				dma_addr = dma_map_single(&adapter->pdev->dev,
+							  skb->data,
+							  skb_headlen(skb),
+							  DMA_TO_DEVICE);
+				desc[frag].addr_lo = dma_addr & 0xFFFFFFFF;
+				desc[frag].addr_hi = dma_addr >> 32;
+				frag++;
 			} else {
-				desc[frag].addr_hi = 0;
 				desc[frag].len_vlan = skb_headlen(skb) / 2;
-
-				/* NOTE: Here, the dma_addr_t returned from
-				 * dma_map_single() is implicitly cast as a
-				 * u32. Although dma_addr_t can be
-				 * 64-bit, the address returned by
-				 * dma_map_single() is always 32-bit
-				 * addressable (as defined by the pci/dma
-				 * subsystem)
-				 */
-				desc[frag++].addr_lo =
-				    dma_map_single(&adapter->pdev->dev,
-						   skb->data,
-						   (skb_headlen(skb) / 2),
-						   DMA_TO_DEVICE);
-				desc[frag].addr_hi = 0;
+				dma_addr = dma_map_single(&adapter->pdev->dev,
+							  skb->data,
+							  (skb_headlen(skb) / 2),
+							  DMA_TO_DEVICE);
+				desc[frag].addr_lo = dma_addr & 0xFFFFFFFF;
+				desc[frag].addr_hi = dma_addr >> 32;
+				frag++;
 
 				desc[frag].len_vlan = skb_headlen(skb) / 2;
-
-				/* NOTE: Here, the dma_addr_t returned from
-				 * dma_map_single() is implicitly cast as a
-				 * u32. Although dma_addr_t can be
-				 * 64-bit, the address returned by
-				 * dma_map_single() is always 32-bit
-				 * addressable (as defined by the pci/dma
-				 * subsystem)
-				 */
-				desc[frag++].addr_lo =
-				    dma_map_single(&adapter->pdev->dev,
-						   skb->data +
-							 (skb_headlen(skb) / 2),
-						   (skb_headlen(skb) / 2),
-						   DMA_TO_DEVICE);
+				dma_addr = dma_map_single(&adapter->pdev->dev,
+							  skb->data +
+							  (skb_headlen(skb) / 2),
+							  (skb_headlen(skb) / 2),
+							  DMA_TO_DEVICE);
+				desc[frag].addr_lo = dma_addr & 0xFFFFFFFF;
+				desc[frag].addr_hi = dma_addr >> 32;
+				frag++;
 			}
 		} else {
-			desc[frag].addr_hi = 0;
-			desc[frag].len_vlan =
-					frags[i - 1].size;
-
-			/* NOTE: Here, the dma_addr_t returned from
-			 * dma_map_page() is implicitly cast as a u32.
-			 * Although dma_addr_t can be 64-bit, the address
-			 * returned by dma_map_page() is always 32-bit
-			 * addressable (as defined by the pci/dma subsystem)
-			 */
-			desc[frag++].addr_lo = skb_frag_dma_map(
-							&adapter->pdev->dev,
-							&frags[i - 1],
-							0,
-							frags[i - 1].size,
-							DMA_TO_DEVICE);
+			desc[frag].len_vlan = frags[i - 1].size;
+			dma_addr = skb_frag_dma_map(&adapter->pdev->dev,
+						    &frags[i - 1],
+						    0,
+						    frags[i - 1].size,
+						    DMA_TO_DEVICE);
+			desc[frag].addr_lo = dma_addr & 0xFFFFFFFF;
+			desc[frag].addr_hi = dma_addr >> 32;
+			frag++;
 		}
 	}
 
@@ -3611,6 +3581,7 @@ static inline void free_send_packet(struct et131x_adapter *adapter,
 	unsigned long flags;
 	struct tx_desc *desc = NULL;
 	struct net_device_stats *stats = &adapter->net_stats;
+	dma_addr_t dma_addr;
 
 	if (tcb->flags & fMP_DEST_BROAD)
 		atomic_inc(&adapter->stats.broadcast_pkts_xmtd);
@@ -3631,8 +3602,13 @@ static inline void free_send_packet(struct et131x_adapter *adapter,
 				    (adapter->tx_ring.tx_desc_ring +
 						INDEX10(tcb->index_start));
 
+			dma_addr = desc->addr_lo;
+
+			if (sizeof(dma_addr_t) == sizeof(u64))
+				dma_addr += ((dma_addr_t)desc->addr_hi) << 32;
+
 			dma_unmap_single(&adapter->pdev->dev,
-					 desc->addr_lo,
+					 dma_addr,
 					 desc->len_vlan, DMA_TO_DEVICE);
 
 			add_10bit(&tcb->index_start, 1);
-- 
1.7.9.5


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

* Re: [PATCH 2/2] staging: et131x: Fix 64bit tx dma address handling
  2012-10-17 21:15 ` [PATCH 2/2] staging: et131x: Fix 64bit tx dma address handling Mark Einon
@ 2012-10-18  0:31   ` Jeffrey Ladouceur
  2012-10-18  9:59     ` Alan Cox
  2012-10-18 20:34   ` [PATCH 2/2 v2] " Mark Einon
  1 sibling, 1 reply; 6+ messages in thread
From: Jeffrey Ladouceur @ 2012-10-18  0:31 UTC (permalink / raw)
  To: Mark Einon; +Cc: devel, gregkh, linux-kernel

On Wed, Oct 17, 2012 at 5:15 PM, Mark Einon <mark.einon@gmail.com> wrote:
> The driver checks that the device can handle 64bit DMA addressing in
> et131x_pci_setup(), but then assumes that the top dword of a tx dma
> address is always zero when creating a dma mapping in nic_send_packet().
> Fix the mapping to use the higher dword of the dma_addr_t returned by
> dma_map_single() and skb_frag_dma_map().
>
> Also remove incorrect comments stating that dma_map_single() only returns
> a 32 bit address.
>
> Signed-off-by: Mark Einon <mark.einon@gmail.com>
> ---
>  drivers/staging/et131x/et131x.c |  102 +++++++++++++++------------------------
>  1 file changed, 39 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
> index f0fe4bd..83643be 100644
> --- a/drivers/staging/et131x/et131x.c
> +++ b/drivers/staging/et131x/et131x.c
> @@ -3285,6 +3285,7 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
>         struct skb_frag_struct *frags = &skb_shinfo(skb)->frags[0];
>         unsigned long flags;
>         struct phy_device *phydev = adapter->phydev;
> +       dma_addr_t dma_addr;
>
>         /* Part of the optimizations of this send routine restrict us to
>          * sending 24 fragments at a pass.  In practice we should never see
> @@ -3314,77 +3315,46 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
>                          * doesn't seem to like large fragments.
>                          */
>                         if (skb_headlen(skb) <= 1514) {
> -                               desc[frag].addr_hi = 0;
>                                 /* Low 16bits are length, high is vlan and
>                                    unused currently so zero */
>                                 desc[frag].len_vlan = skb_headlen(skb);
> -
> -                               /* NOTE: Here, the dma_addr_t returned from
> -                                * dma_map_single() is implicitly cast as a
> -                                * u32. Although dma_addr_t can be
> -                                * 64-bit, the address returned by
> -                                * dma_map_single() is always 32-bit
> -                                * addressable (as defined by the pci/dma
> -                                * subsystem)
> -                                */
> -                               desc[frag++].addr_lo =
> -                                   dma_map_single(&adapter->pdev->dev,
> -                                                  skb->data,
> -                                                  skb_headlen(skb),
> -                                                  DMA_TO_DEVICE);
> +                               dma_addr = dma_map_single(&adapter->pdev->dev,
> +                                                         skb->data,
> +                                                         skb_headlen(skb),
> +                                                         DMA_TO_DEVICE);
> +                               desc[frag].addr_lo = dma_addr & 0xFFFFFFFF;
> +                               desc[frag].addr_hi = dma_addr >> 32;

Maybe use macros defined in kernel.h instead:

desc[frag].addr_lo = lower_32_bits(dma_addr);
desc[frag].addr_hi = upper_32_bits(dma_addr);

A few more instances below.


> +                               frag++;
>                         } else {
> -                               desc[frag].addr_hi = 0;
>                                 desc[frag].len_vlan = skb_headlen(skb) / 2;
> -
> -                               /* NOTE: Here, the dma_addr_t returned from
> -                                * dma_map_single() is implicitly cast as a
> -                                * u32. Although dma_addr_t can be
> -                                * 64-bit, the address returned by
> -                                * dma_map_single() is always 32-bit
> -                                * addressable (as defined by the pci/dma
> -                                * subsystem)
> -                                */
> -                               desc[frag++].addr_lo =
> -                                   dma_map_single(&adapter->pdev->dev,
> -                                                  skb->data,
> -                                                  (skb_headlen(skb) / 2),
> -                                                  DMA_TO_DEVICE);
> -                               desc[frag].addr_hi = 0;
> +                               dma_addr = dma_map_single(&adapter->pdev->dev,
> +                                                         skb->data,
> +                                                         (skb_headlen(skb) / 2),
> +                                                         DMA_TO_DEVICE);
> +                               desc[frag].addr_lo = dma_addr & 0xFFFFFFFF;
> +                               desc[frag].addr_hi = dma_addr >> 32;
> +                               frag++;
>
>                                 desc[frag].len_vlan = skb_headlen(skb) / 2;
> -
> -                               /* NOTE: Here, the dma_addr_t returned from
> -                                * dma_map_single() is implicitly cast as a
> -                                * u32. Although dma_addr_t can be
> -                                * 64-bit, the address returned by
> -                                * dma_map_single() is always 32-bit
> -                                * addressable (as defined by the pci/dma
> -                                * subsystem)
> -                                */
> -                               desc[frag++].addr_lo =
> -                                   dma_map_single(&adapter->pdev->dev,
> -                                                  skb->data +
> -                                                        (skb_headlen(skb) / 2),
> -                                                  (skb_headlen(skb) / 2),
> -                                                  DMA_TO_DEVICE);
> +                               dma_addr = dma_map_single(&adapter->pdev->dev,
> +                                                         skb->data +
> +                                                         (skb_headlen(skb) / 2),
> +                                                         (skb_headlen(skb) / 2),
> +                                                         DMA_TO_DEVICE);
> +                               desc[frag].addr_lo = dma_addr & 0xFFFFFFFF;
> +                               desc[frag].addr_hi = dma_addr >> 32;
> +                               frag++;
>                         }
>                 } else {
> -                       desc[frag].addr_hi = 0;
> -                       desc[frag].len_vlan =
> -                                       frags[i - 1].size;
> -
> -                       /* NOTE: Here, the dma_addr_t returned from
> -                        * dma_map_page() is implicitly cast as a u32.
> -                        * Although dma_addr_t can be 64-bit, the address
> -                        * returned by dma_map_page() is always 32-bit
> -                        * addressable (as defined by the pci/dma subsystem)
> -                        */
> -                       desc[frag++].addr_lo = skb_frag_dma_map(
> -                                                       &adapter->pdev->dev,
> -                                                       &frags[i - 1],
> -                                                       0,
> -                                                       frags[i - 1].size,
> -                                                       DMA_TO_DEVICE);
> +                       desc[frag].len_vlan = frags[i - 1].size;
> +                       dma_addr = skb_frag_dma_map(&adapter->pdev->dev,
> +                                                   &frags[i - 1],
> +                                                   0,
> +                                                   frags[i - 1].size,
> +                                                   DMA_TO_DEVICE);
> +                       desc[frag].addr_lo = dma_addr & 0xFFFFFFFF;
> +                       desc[frag].addr_hi = dma_addr >> 32;
> +                       frag++;
>                 }
>         }
>
> @@ -3611,6 +3581,7 @@ static inline void free_send_packet(struct et131x_adapter *adapter,
>         unsigned long flags;
>         struct tx_desc *desc = NULL;
>         struct net_device_stats *stats = &adapter->net_stats;
> +       dma_addr_t dma_addr;
>
>         if (tcb->flags & fMP_DEST_BROAD)
>                 atomic_inc(&adapter->stats.broadcast_pkts_xmtd);
> @@ -3631,8 +3602,13 @@ static inline void free_send_packet(struct et131x_adapter *adapter,
>                                     (adapter->tx_ring.tx_desc_ring +
>                                                 INDEX10(tcb->index_start));
>
> +                       dma_addr = desc->addr_lo;
> +
> +                       if (sizeof(dma_addr_t) == sizeof(u64))
> +                               dma_addr += ((dma_addr_t)desc->addr_hi) << 32;

Probably end result the same, but don't see + too often. Not usually
the way we think of it.

dma_addr |= (dma_addr_t)desc->addr_hi << 32;

> +
>                         dma_unmap_single(&adapter->pdev->dev,
> -                                        desc->addr_lo,
> +                                        dma_addr,
>                                          desc->len_vlan, DMA_TO_DEVICE);
>
>                         add_10bit(&tcb->index_start, 1);
> --
> 1.7.9.5
>
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

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

* Re: [PATCH 2/2] staging: et131x: Fix 64bit tx dma address handling
  2012-10-18  0:31   ` Jeffrey Ladouceur
@ 2012-10-18  9:59     ` Alan Cox
  2012-10-18 10:17       ` Mark Einon
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Cox @ 2012-10-18  9:59 UTC (permalink / raw)
  To: Jeffrey Ladouceur; +Cc: Mark Einon, devel, gregkh, linux-kernel

> > +                                                         skb_headlen(skb),
> > +                                                         DMA_TO_DEVICE);
> > +                               desc[frag].addr_lo = dma_addr & 0xFFFFFFFF;
> > +                               desc[frag].addr_hi = dma_addr >> 32;
> 
> Maybe use macros defined in kernel.h instead:
> 
> desc[frag].addr_lo = lower_32_bits(dma_addr);
> desc[frag].addr_hi = upper_32_bits(dma_addr);
> 
> A few more instances below.

This is actually important because >> 32 of a 32bit value is undefined in
C. The compiler is free to do what it likes with this. While the results
are usually sane some architectures do generate the equivalent of

		x >> (n % wordsize);


Alan

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

* Re: [PATCH 2/2] staging: et131x: Fix 64bit tx dma address handling
  2012-10-18  9:59     ` Alan Cox
@ 2012-10-18 10:17       ` Mark Einon
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Einon @ 2012-10-18 10:17 UTC (permalink / raw)
  To: Alan Cox, Jeffrey Ladouceur; +Cc: devel, gregkh, linux-kernel

On Thu, Oct 18, 2012 at 10:59:56AM +0100, Alan Cox wrote:
> > > +                                                         skb_headlen(skb),
> > > +                                                         DMA_TO_DEVICE);
> > > +                               desc[frag].addr_lo = dma_addr & 0xFFFFFFFF;
> > > +                               desc[frag].addr_hi = dma_addr >> 32;
> > 
> > Maybe use macros defined in kernel.h instead:
> > 
> > desc[frag].addr_lo = lower_32_bits(dma_addr);
> > desc[frag].addr_hi = upper_32_bits(dma_addr);
> > 
> > A few more instances below.
> 
> This is actually important because >> 32 of a 32bit value is undefined in
> C. The compiler is free to do what it likes with this. While the results
> are usually sane some architectures do generate the equivalent of
> 
> 		x >> (n % wordsize);
> 
> Alan

Thanks for the comments both. I'll send an updated patch this evening.

The intention is to eventually store this value as a dma_addr_t anyway,
but some more work needs to be done to copy the address correctly to the
desc ring.

Cheers,

Mark

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

* [PATCH 2/2 v2] staging: et131x: Fix 64bit tx dma address handling
  2012-10-17 21:15 ` [PATCH 2/2] staging: et131x: Fix 64bit tx dma address handling Mark Einon
  2012-10-18  0:31   ` Jeffrey Ladouceur
@ 2012-10-18 20:34   ` Mark Einon
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Einon @ 2012-10-18 20:34 UTC (permalink / raw)
  To: devel, gregkh; +Cc: linux-kernel, Mark Einon

The driver checks that the device can handle 64bit DMA addressing in
et131x_pci_setup(), but then assumes that the top dword of a tx dma
address is always zero when creating a dma mapping in nic_send_packet().
Fix the mapping to use the higher dword of the dma_addr_t returned by
dma_map_single() and skb_frag_dma_map().

Also remove incorrect comments stating that dma_map_single() only returns
a 32 bit address.

Signed-off-by: Mark Einon <mark.einon@gmail.com>
---
 drivers/staging/et131x/et131x.c |  102 +++++++++++++++------------------------
 1 file changed, 39 insertions(+), 63 deletions(-)

diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index 9d258a3..23d166b 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -3285,6 +3285,7 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
 	struct skb_frag_struct *frags = &skb_shinfo(skb)->frags[0];
 	unsigned long flags;
 	struct phy_device *phydev = adapter->phydev;
+	dma_addr_t dma_addr;
 
 	/* Part of the optimizations of this send routine restrict us to
 	 * sending 24 fragments at a pass.  In practice we should never see
@@ -3314,77 +3315,46 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
 			 * doesn't seem to like large fragments.
 			 */
 			if (skb_headlen(skb) <= 1514) {
-				desc[frag].addr_hi = 0;
 				/* Low 16bits are length, high is vlan and
 				   unused currently so zero */
 				desc[frag].len_vlan = skb_headlen(skb);
-
-				/* NOTE: Here, the dma_addr_t returned from
-				 * dma_map_single() is implicitly cast as a
-				 * u32. Although dma_addr_t can be
-				 * 64-bit, the address returned by
-				 * dma_map_single() is always 32-bit
-				 * addressable (as defined by the pci/dma
-				 * subsystem)
-				 */
-				desc[frag++].addr_lo =
-				    dma_map_single(&adapter->pdev->dev,
-						   skb->data,
-						   skb_headlen(skb),
-						   DMA_TO_DEVICE);
+				dma_addr = dma_map_single(&adapter->pdev->dev,
+							  skb->data,
+							  skb_headlen(skb),
+							  DMA_TO_DEVICE);
+				desc[frag].addr_lo = lower_32_bits(dma_addr);
+				desc[frag].addr_hi = upper_32_bits(dma_addr);
+				frag++;
 			} else {
-				desc[frag].addr_hi = 0;
 				desc[frag].len_vlan = skb_headlen(skb) / 2;
-
-				/* NOTE: Here, the dma_addr_t returned from
-				 * dma_map_single() is implicitly cast as a
-				 * u32. Although dma_addr_t can be
-				 * 64-bit, the address returned by
-				 * dma_map_single() is always 32-bit
-				 * addressable (as defined by the pci/dma
-				 * subsystem)
-				 */
-				desc[frag++].addr_lo =
-				    dma_map_single(&adapter->pdev->dev,
-						   skb->data,
-						   (skb_headlen(skb) / 2),
-						   DMA_TO_DEVICE);
-				desc[frag].addr_hi = 0;
+				dma_addr = dma_map_single(&adapter->pdev->dev,
+							  skb->data,
+							  (skb_headlen(skb) / 2),
+							  DMA_TO_DEVICE);
+				desc[frag].addr_lo = lower_32_bits(dma_addr);
+				desc[frag].addr_hi = upper_32_bits(dma_addr);
+				frag++;
 
 				desc[frag].len_vlan = skb_headlen(skb) / 2;
-
-				/* NOTE: Here, the dma_addr_t returned from
-				 * dma_map_single() is implicitly cast as a
-				 * u32. Although dma_addr_t can be
-				 * 64-bit, the address returned by
-				 * dma_map_single() is always 32-bit
-				 * addressable (as defined by the pci/dma
-				 * subsystem)
-				 */
-				desc[frag++].addr_lo =
-				    dma_map_single(&adapter->pdev->dev,
-						   skb->data +
-							 (skb_headlen(skb) / 2),
-						   (skb_headlen(skb) / 2),
-						   DMA_TO_DEVICE);
+				dma_addr = dma_map_single(&adapter->pdev->dev,
+							  skb->data +
+							  (skb_headlen(skb) / 2),
+							  (skb_headlen(skb) / 2),
+							  DMA_TO_DEVICE);
+				desc[frag].addr_lo = lower_32_bits(dma_addr);
+				desc[frag].addr_hi = upper_32_bits(dma_addr);
+				frag++;
 			}
 		} else {
-			desc[frag].addr_hi = 0;
-			desc[frag].len_vlan =
-					frags[i - 1].size;
-
-			/* NOTE: Here, the dma_addr_t returned from
-			 * dma_map_page() is implicitly cast as a u32.
-			 * Although dma_addr_t can be 64-bit, the address
-			 * returned by dma_map_page() is always 32-bit
-			 * addressable (as defined by the pci/dma subsystem)
-			 */
-			desc[frag++].addr_lo = skb_frag_dma_map(
-							&adapter->pdev->dev,
-							&frags[i - 1],
-							0,
-							frags[i - 1].size,
-							DMA_TO_DEVICE);
+			desc[frag].len_vlan = frags[i - 1].size;
+			dma_addr = skb_frag_dma_map(&adapter->pdev->dev,
+						    &frags[i - 1],
+						    0,
+						    frags[i - 1].size,
+						    DMA_TO_DEVICE);
+			desc[frag].addr_lo = lower_32_bits(dma_addr);
+			desc[frag].addr_hi = upper_32_bits(dma_addr);
+			frag++;
 		}
 	}
 
@@ -3611,6 +3581,7 @@ static inline void free_send_packet(struct et131x_adapter *adapter,
 	unsigned long flags;
 	struct tx_desc *desc = NULL;
 	struct net_device_stats *stats = &adapter->net_stats;
+	dma_addr_t dma_addr;
 
 	if (tcb->flags & fMP_DEST_BROAD)
 		atomic_inc(&adapter->stats.broadcast_pkts_xmtd);
@@ -3631,8 +3602,13 @@ static inline void free_send_packet(struct et131x_adapter *adapter,
 				    (adapter->tx_ring.tx_desc_ring +
 						INDEX10(tcb->index_start));
 
+			dma_addr = desc->addr_lo;
+
+			if (sizeof(dma_addr_t) == sizeof(u64))
+				dma_addr |= ((dma_addr_t)desc->addr_hi) << 32;
+
 			dma_unmap_single(&adapter->pdev->dev,
-					 desc->addr_lo,
+					 dma_addr,
 					 desc->len_vlan, DMA_TO_DEVICE);
 
 			add_10bit(&tcb->index_start, 1);
-- 
1.7.9.5


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

end of thread, other threads:[~2012-10-18 20:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-17 21:15 [PATCH 1/2] staging: et131x: Use skb_headlen() where appropriate Mark Einon
2012-10-17 21:15 ` [PATCH 2/2] staging: et131x: Fix 64bit tx dma address handling Mark Einon
2012-10-18  0:31   ` Jeffrey Ladouceur
2012-10-18  9:59     ` Alan Cox
2012-10-18 10:17       ` Mark Einon
2012-10-18 20:34   ` [PATCH 2/2 v2] " Mark Einon

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