On Thu, Feb 7, 2013 at 6:11 PM, Wolfgang Denk <w...@denx.de> wrote: > Dear Sonic Zhang, > > In message <1360223258-6945-5-git-send-email-sonic....@gmail.com> you wrote: >> From: Bob Liu <lliu...@gmail.com> >> >> Add dma support for bf60x. >> >> Signed-off-by: Bob Liu <lliu...@gmail.com> >> Signed-off-by: Sonic Zhang <sonic.zh...@analog.com> >> --- >> arch/blackfin/include/asm/dma.h | 113 >> ++++++++++++---------- >> arch/blackfin/include/asm/mach-common/bits/dma.h | 48 +++++++++- >> arch/blackfin/lib/string.c | 97 ++++++++++--------- >> 3 files changed, 159 insertions(+), 99 deletions(-) > > There are checkpatch errors and warnings. Please fix these!! >
No problem. > >> struct dmasg_large { >> void *next_desc_addr; >> - unsigned long start_addr; >> - unsigned short cfg; >> - unsigned short x_count; >> - short x_modify; >> - unsigned short y_count; >> - short y_modify; >> + u32 start_addr; >> + u16 cfg; >> + u16 x_count; >> + s16 x_modify; >> + u16 y_count; >> + s16 y_modify; >> } __attribute__((packed)); >> >> struct dmasg { >> - unsigned long start_addr; >> - unsigned short cfg; >> - unsigned short x_count; >> - short x_modify; >> - unsigned short y_count; >> - short y_modify; >> + u32 start_addr; >> + u16 cfg; >> + u16 x_count; >> + s16 x_modify; >> + u16 y_count; >> + s16 y_modify; >> } __attribute__((packed)); > > These appear totally unrelated changes. These should be split out > into separate patches. > OK, I will move these changes out as a new patch. >> +/* >> + * All Blackfin system MMRs are padded to 32bits even if the register >> + * itself is only 16bits. So use a helper macro to streamline this. >> + */ >> +#define __BFP(m) m; u16 __pad_##m >> struct dma_register { >> - void *next_desc_ptr; /* DMA Next Descriptor Pointer register */ >> - unsigned long start_addr; /* DMA Start address register */ >> - >> - unsigned short cfg; /* DMA Configuration register */ >> - unsigned short dummy1; /* DMA Configuration register */ >> - >> - unsigned long reserved; >> - >> - unsigned short x_count; /* DMA x_count register */ >> - unsigned short dummy2; >> - >> - short x_modify; /* DMA x_modify register */ >> - unsigned short dummy3; >> - >> - unsigned short y_count; /* DMA y_count register */ >> - unsigned short dummy4; >> - >> - short y_modify; /* DMA y_modify register */ >> - unsigned short dummy5; >> - >> - void *curr_desc_ptr; /* DMA Current Descriptor Pointer >> - register */ >> - unsigned long curr_addr_ptr; /* DMA Current Address Pointer >> - register */ >> - unsigned short irq_status; /* DMA irq status register */ >> - unsigned short dummy6; >> - >> - unsigned short peripheral_map; /* DMA peripheral map register */ >> - unsigned short dummy7; >> - >> - unsigned short curr_x_count; /* DMA Current x-count register */ >> - unsigned short dummy8; >> - >> - unsigned long reserved2; >> - >> - unsigned short curr_y_count; /* DMA Current y-count register */ >> - unsigned short dummy9; >> - >> - unsigned long reserved3; >> - >> +#ifdef __ADSPBF60x__ >> + void *next_desc_ptr; >> + u32 start_addr; >> + u32 config; > > You drop all the comments; please don't do that. > > Also, "cfg" used to be a short, now it gets replaced by "config" which > is a u32. Is this correct? Yes, this is correct for BF60x. The DMA config MMR on BF60x is 32 bits while it is 16 bits on BF5xx. > >> + void *next_desc_ptr; >> + u32 start_addr; >> + u16 __BFP(config); >> + u32 __pad0; >> + u16 __BFP(x_count); >> + s16 __BFP(x_modify); >> + u16 __BFP(y_count); >> + s16 __BFP(y_modify); >> + void *curr_desc_ptr; >> + u32 curr_addr_ptr; >> + u16 __BFP(status); >> + u16 __BFP(peripheral_map); >> + u16 __BFP(curr_x_count); >> + u32 __pad1; >> + u16 __BFP(curr_y_count); >> + u32 __pad2; > > Sorry, but this is unreadable. NAK. > OK, I will change back to the former definition style. Sonic Zhang _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot