[U-Boot] [PATCH 04/11] Blackfin: bf60x: add dma support

Wolfgang Denk
2013-02-07T10:11:08Z

Dear Sonic Zhang,

In message <1360223258-6945-5-git-send-email-sonic.adi@gmail.com> you wrote:
> From: Bob Liu <lliubbo@gmail.com>
>
> Add dma support for bf60x.
>
> Signed-off-by: Bob Liu <lliubbo@gmail.com>
> Signed-off-by: Sonic Zhang <sonic.zhang@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!!

> 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.

> +/*
> + * 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?

> + 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.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Software suppliers are trying to make their software packages more
``user-friendly''. . . . Their best approach, so far, has been to
take all the old brochures, and stamp the words, ``user-friendly'' on
the cover. - Bill Gates