(+ Alan)
On 23 September 2016 at 00:49, Daniil Egranov daniil.egranov@arm.com wrote:
- Corrected Ethernet frame size to 1538.
- Performance optimization. Removed extra CopyMem() call.
Could you split this in 2 patches please?
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Daniil Egranov daniil.egranov@arm.com
NOTE: The driver requires [edk2] [PATCH 0/2] Juno PCI fixes
Drivers/Net/MarvellYukonDxe/if_msk.c | 22 +++++++++------------- Drivers/Net/MarvellYukonDxe/if_msk.h | 2 +- 2 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/Drivers/Net/MarvellYukonDxe/if_msk.c b/Drivers/Net/MarvellYukonDxe/if_msk.c index 6fb8af4..8e9b726 100644 --- a/Drivers/Net/MarvellYukonDxe/if_msk.c +++ b/Drivers/Net/MarvellYukonDxe/if_msk.c @@ -1876,7 +1876,6 @@ msk_rxeof ( struct msk_rxdesc *rxd; INTN cons; INTN rxlen;
MSK_DMA_BUF m;
DEBUG ((EFI_D_NET, "Marvell Yukon: rxeof\n"));
@@ -1906,16 +1905,6 @@ msk_rxeof ( rxd = &sc_if->msk_cdata.msk_rxdesc[cons]; #endif
- gBS->CopyMem (&m, &rxd->rx_m, sizeof(m));
- Status = msk_newbuf (sc_if, cons);
- if (EFI_ERROR (Status)) {
// This is a dropped packet, but we aren't counting drops
// Reuse old buffer
msk_discard_rxbuf (sc_if, cons);
What happened to dropping the packet in this case?
If you want to get rid of the gBS->CopyMem(), why not simply assign 'rxd->rx_m' to 'm' field by field? That should help the compiler optimize away the in-memory version of 'm' entirely
break;
- }
- Status = mPciIo->Flush (mPciIo); if (EFI_ERROR (Status)) { DEBUG ((EFI_D_NET, "Marvell Yukon: failed to Flush DMA\n"));
@@ -1932,14 +1921,21 @@ msk_rxeof ( if (!EFI_ERROR (Status)) { gBS->SetMem (m_link, sizeof (MSK_LINKED_SYSTEM_BUF), 0); m_link->Signature = RX_MBUF_SIGNATURE;
m_link->SystemBuf.Buf = m.Buf;
m_link->SystemBuf.Buf = rxd->rx_m.Buf; m_link->SystemBuf.Length = len; InsertTailList (&mSoftc->ReceiveQueueHead, &m_link->Link);
} else { DEBUG ((EFI_D_NET, "Marvell Yukon: failed to allocate receive buffer link. Dropping Frame\n"));
gBS->FreePool (m.Buf);
gBS->FreePool (rxd->rx_m.Buf);
}
Status = msk_newbuf (sc_if, cons);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_NET, "Marvell Yukon: failed to allocate DMA buffer\n"));
break;
}
} while (0);
MSK_RX_INC (sc_if->msk_cdata.msk_rx_cons, MSK_RX_RING_CNT);
diff --git a/Drivers/Net/MarvellYukonDxe/if_msk.h b/Drivers/Net/MarvellYukonDxe/if_msk.h index 66513d3..64ddeac 100644 --- a/Drivers/Net/MarvellYukonDxe/if_msk.h +++ b/Drivers/Net/MarvellYukonDxe/if_msk.h @@ -15,7 +15,7 @@
#include <Uefi.h>
-#define MAX_SUPPORTED_PACKET_SIZE (1500) +#define MAX_SUPPORTED_PACKET_SIZE (1538)
This looks reasonable, but could you still explain why you need to make this change?
EFI_STATUS mskc_probe (EFI_PCI_IO_PROTOCOL *PciIo);
-- 2.7.4