lkml.org 
[lkml]   [2011]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/4] staging: et131x: Convert rest of pci memory management to dma api
Mark Einon <mark.einon@gmail.com> :
> Replaced pci map/unmap and set_mask calls with their dma equivalents.
> Also updated comments to reflect this.
>
> Signed-off-by: Mark Einon <mark.einon@gmail.com>
> ---
> drivers/staging/et131x/et131x.c | 56 +++++++++++++++++++-------------------
> 1 files changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
> index 993f93a..1c11dfd 100644
> --- a/drivers/staging/et131x/et131x.c
> +++ b/drivers/staging/et131x/et131x.c
> @@ -3202,59 +3202,59 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
[...]
> desc[frag++].addr_lo =
> - pci_map_single(adapter->pdev,
> + dma_map_single(&adapter->pdev->dev,
> skb->data,
> skb->len -
> skb->data_len,
> - PCI_DMA_TODEVICE);
> + DMA_TO_DEVICE);
> } else {

- Some dma_mapping_error() would be welcome.

- (nit) If you keep repeating &adapter->pdev->dev, you may consider adding
some local variable

- could you rework the Tx path as well so that despite the cascade of
method the code does not end fighting for the right end of the screen ?
et131x_tx is almost empty. It could make some sense to merge it with
its callee (and rename it et131x_start_xmit ?).

- nic_send_packet
for (i = 0; i < nr_frags; i++) {
[...comment..]
if (i == 0) {
Lovely...

[big block]
} else {
[small block]
}
... really, really lovely.

}
[...]
> /* NOTE: Here, the dma_addr_t returned from
> - * pci_map_single() is implicitly cast as a
> + * dma_map_single() is implicitly cast as a
> * u32. Although dma_addr_t can be
> * 64-bit, the address returned by
> - * pci_map_single() is always 32-bit
> + * dma_map_single() is always 32-bit
> * addressable (as defined by the pci/dma
> * subsystem)
> */
> desc[frag++].addr_lo =
[...]
> desc[frag].addr_hi = 0;

- The NOTE seems a bit outdated. Afaiks the driver tries to set a 64 bits
wide DMA mask. Both addr_lo and addr_hi should be set with the returned
mapping and/or even replaced with a single 64 bits addr field in tx_desc.

- Speaking of it, tx_desc probably lacks some __leXY annotations.

- (nic_send_packet)
{
u32 i;
struct tx_desc desc[24]; /* 24 x 16 byte */
384 bytes. :o/

--
Ueimor


\
 
 \ /
  Last update: 2011-10-16 19:01    [from the cache]
©2003-2011 Jasper Spaans