On Sat, Jun 22, 2013 at 10:42 PM, Tomasz Figa tomasz.figa@gmail.com wrote:
+enum {
PL080_LLI_SRC,PL080_LLI_DST,PL080_LLI_LLI,PL080_LLI_CCTL,PL080_LLI_WORDS+};
I usually don't like it when enums are not given names, and are implicitly cast to integers.
I think it'd be better to just use #define for these so we know what is going on.
@@ -181,7 +177,7 @@ struct pl08x_txd { struct virt_dma_desc vd; struct list_head dsg_list; dma_addr_t llis_bus;
struct pl08x_lli *llis_va;
u32 *llis_va;
It's nice that you use the u32 * here for proper indexing into an array.
-/* Size (bytes) of each LLI buffer allocated for one transfer */ -# define PL08X_LLI_TSFR_SIZE 0x2000
-/* Maximum times we call dma_pool_alloc on this pool without freeing */ -#define MAX_NUM_TSFR_LLIS (PL08X_LLI_TSFR_SIZE/sizeof(struct pl08x_lli)) +/*
- Number of LLIs in each LLI buffer allocated for one transfer
- (maximum times we call dma_pool_alloc on this pool without freeing)
- */
+#define MAX_NUM_TSFR_LLIS 512
And I like this nice side effect that we allocate a number of LLIs rather than a fixed-size buffer.
+static void pl08x_write_lli(struct pl08x_driver_data *pl08x,
struct pl08x_phy_chan *phychan, const u32 *lli, u32 ccfg)+{
dev_vdbg(&pl08x->adev->dev,"WRITE channel %d: csrc=0x%08x, cdst=0x%08x, ""clli=0x%08x, cctl=0x%08x, ccfg=0x%08x\n",phychan->id, lli[PL080_LLI_SRC], lli[PL080_LLI_DST],lli[PL080_LLI_LLI], lli[PL080_LLI_CCTL], ccfg);writel(lli[PL080_LLI_SRC], phychan->base + PL080_CH_SRC_ADDR);writel(lli[PL080_LLI_DST], phychan->base + PL080_CH_DST_ADDR);writel(lli[PL080_LLI_LLI], phychan->base + PL080_CH_LLI);writel(lli[PL080_LLI_CCTL], phychan->base + PL080_CH_CONTROL);writel(ccfg, phychan->reg_config);
Take this opportunity to replace the first four writel() with writel_relaxed(), keep the last one to make sure all hit the hardware.
Apart from that this looks nice!
Yours, Linus Walleij