Dear Arnd
On Fri, Mar 21, 2014 at 3:37 PM, Arnd Bergmann <[email protected]> wrote:
>> >> >> +static int hip04_alloc_ring(struct net_device *ndev, struct device *d)
>> >> >> +{
>> >> >> + struct hip04_priv *priv = netdev_priv(ndev);
>> >> >> + int i;
>> >> >> +
>> >> >> + priv->rx_buf_size = RX_BUF_SIZE +
>> >> >> + SKB_DATA_ALIGN(sizeof(struct
>> >> >> skb_shared_info));
>> >> >> +
>> >> >> + priv->desc_pool = dma_pool_create(DRV_NAME, d, sizeof(struct
>> >> >> tx_desc),
>> >> >> + SKB_DATA_ALIGN(sizeof(struct tx_desc)),
>> >> >> 0);
>> >> >> + if (!priv->desc_pool)
>> >> >> + return -ENOMEM;
>> >> >> +
>> >> >> + for (i = 0; i < TX_DESC_NUM; i++) {
>> >> >> + priv->td_ring[i] = dma_pool_alloc(priv->desc_pool,
>> >> >> + GFP_ATOMIC, &priv->td_phys[i]);
>> >> >> + if (!priv->td_ring[i])
>> >> >> + return -ENOMEM;
>> >> >> + }
>> >> >
>> >> > Why do you create a dma pool here, when you do all the allocations
>> >> > upfront?
>> >> >
>> >> > It looks to me like you could simply turn the td_ring array of pointers
>> >> > to tx descriptors into a an array of tx descriptors (no pointers) and
>> >> > allocate
>> >> > that one using dma_alloc_coherent.
>> >>
>> >> dma pool used here mainly because of alignment,
>> >> the desc has requirement of SKB_DATA_ALIGN,
>> >> so use the simplest way
>> >> priv->desc_pool = dma_pool_create(DRV_NAME, d, sizeof(struct
>> >> tx_desc),
>> >> SKB_DATA_ALIGN(sizeof(struct tx_desc)),
>> >> 0);
>> >
>> > dma_alloc_coherent() will actually give you PAGE_SIZE alignment, so that's
>> > still easier.
>> However since the alignment requirement, it can not simply use desc[i]
>> to get each desc.
>> desc = dma_alloc_coherent(d, size, &phys, GFP_KERNEL);
>> desc[i] is not what we want.
>> So still prefer using dma_pool_alloc here.
>
> Ah, I see what you mean: struct tx_desc is actually smaller than the
> required alignment. You can fix that by marking the structure
> "____cacheline_aligned".
>
Yes, after recheck the method carefully, it works with __aligned(0x40).
"____cacheline_aligned" is much better.
The desc can be get with either table or pointer.
Will update accordingly, it is simpler.
Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
