On 2018-10-03 14:36, David Laight wrote:
From: Ben Dooks
Sent: 02 October 2018 17:56
The smsc95xx_tx_fixup is doing multiple calls to skb_push() to
put an 8-byte command header onto the packet. It would be easier
to do one skb_push() and then copy the data in once the push is
done.
Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx>
---
drivers/net/usb/smsc95xx.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index cb19aea139d3..813ab93ee2c3 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -2006,6 +2006,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct
usbnet *dev,
bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM :
SMSC95XX_TX_OVERHEAD;
u32 tx_cmd_a, tx_cmd_b;
+ void *ptr;
It might be useful to define a structure for the header.
You might need to find the 'store unaligned 32bit word' macro though.
(Actually that will probably be better than the memcpy() which might
end up doing memory-memory copies rather than storing the register.)
Although if/when you add the tx alignment that won't be needed because
the
header will be aligned.
Ok, might be worth doing.
I did try to do a "u32 tx_cmd[2]" but the code generated ended up
storing
stuff onto the stack before copying into the packet. I agree that
possibly
going to the "put_unaligned" function might be nicer too.
If we did enable tx-align all the time then we'd not have to care about
the
alignment, but I didn't want to do that if possible as that would end up
sending up to 3 bytes extra per packet.
I am trying not too do too many changes at one time to allow roll back.
/* We do not advertise SG, so skbs should be already linearized */
BUG_ON(skb_shinfo(skb)->nr_frags);
@@ -2019,6 +2020,9 @@ static struct sk_buff *smsc95xx_tx_fixup(struct
usbnet *dev,
return NULL;
}
+ tx_cmd_b = (u32)skb->len;
+ tx_cmd_a = tx_cmd_b | TX_CMD_A_FIRST_SEG_ | TX_CMD_A_LAST_SEG_;
+
if (csum) {
if (skb->len <= 45) {
/* workaround - hardware tx checksum does not work
@@ -2035,21 +2039,18 @@ static struct sk_buff
*smsc95xx_tx_fixup(struct usbnet *dev,
skb_push(skb, 4);
cpu_to_le32s(&csum_preamble);
Not related, but csum_preamble = cpu_to_le32(csum_preamble) is likely
to
generate better code (at least for some architectures).
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes,
MK1 1PT, UK
Registration No: 1397386 (Wales)
