Re: [PATCH 3/3] net: hisilicon: new hip04 ethernet driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Dear Arnd

On Fri, Mar 21, 2014 at 3:37 PM, Arnd Bergmann <arnd@xxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux