staging: et131x: Use skb_headlen() where appropriate
PATCH 1/2 staging: et131x: Use skb_headlen() where appropriate
(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 <[email protected]>
--- drivers/staging/et131x/et131x.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-)
drivers/staging/et131x/et131x.c +9 -16
@@ -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) &&
Signature · 1 line
1.7.9.5
PATCH 2/2 staging: et131x: Fix 64bit tx dma address handling
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 <[email protected]>
--- drivers/staging/et131x/et131x.c | 102 +++++++++++++++------------------------ 1 file changed, 39 insertions(+), 63 deletions(-)
drivers/staging/et131x/et131x.c +39 -63
@@ -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 );
Signature · 1 line
1.7.9.5
Re: [PATCH 2/2] staging: et131x: Fix 64bit tx dma address handling
Quoted 44 lines · 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 innic_send_packet(). Fix the mapping to use the higher dword of thedma_addr_treturned bydma_map_single()andskb_frag_dma_map().Also remove incorrect comments stating that
dma_map_single()only returns a 32 bit address.Signed-off-by Mark Einon <[email protected]>--- drivers/staging/et131x/et131x.c | 102 +++++++++++++++------------------------ 1 file changed, 39 insertions(+), 63 deletions(-)drivers/staging/et131x/et131x.c +1
-11@@ -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 =
drivers/staging/et131x/et131x.c static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb) · line 3326
Quoted 10 lines
drivers/staging/et131x/et131x.c +6
-4-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.
Quoted 90 lines
drivers/staging/et131x/et131x.c +26
-47+ 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);
drivers/staging/et131x/et131x.c static inline void free_send_packet(struct et131x_adapter *adapter, · line 3608
Quoted 7 lines
drivers/staging/et131x/et131x.c +4
-0@@ -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;
Quoted 14 lines
drivers/staging/et131x/et131x.c +2
-2+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
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
Re: [PATCH 2/2] staging: et131x: Fix 64bit tx dma address handling
Quoted 11 lines
drivers/staging/et131x/et131x.c static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb) · line 3326
Quoted 4 lines
drivers/staging/et131x/et131x.c +4
-0+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.hinstead: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
Re: [PATCH 2/2] staging: et131x: Fix 64bit tx dma address handling
Quoted 12 lines · Alan Cox
Quoted 11 lines
drivers/staging/et131x/et131x.c static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb) · line 3326
Quoted 4 lines
drivers/staging/et131x/et131x.c +4
-0+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.hinstead:desc[frag].addr_lo =
lower_32_bits(dma_addr);
desc[frag].addr_hi =upper_32_bits(dma_addr);A few more instances below.
Quoted 7 lines
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
PATCH v2 2/2 staging: et131x: Fix 64bit tx dma address handling
In reply to Mark EinonThe 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 <[email protected]>
--- drivers/staging/et131x/et131x.c | 102 +++++++++++++++------------------------ 1 file changed, 39 insertions(+), 63 deletions(-)
drivers/staging/et131x/et131x.c +39 -63
@@ -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 );
Signature · 1 line
1.7.9.5