[U-Boot] [PATCH] S5P: new spi gpio bitbang driver

[Mike Frysinger]

On Tuesday, September 14, 2010 03:38:11 Donghwa Lee wrote:
> This patch adds basic support for spi mode 0~3 by control gpio bitbang in
> S5P. Original name of this patch was "support spi gpio driver by control
> gpio bitbang". But, it had arch-specific features, S5P, so changed to this
> name.

so why arent you implementing this with the common spi API ?  then any of the 
code in the tree would be able to use this spi driver without having to change 
to your arch-specific API.

> +++ b/arch/arm/include/asm/arch-s5pc1xx/spi.h
> @@ -0,0 +1,53 @@
> +
> +#ifndef __ASM_ARCH_SPI_H_
> +#define __ASM_ARCH_SPI_H_
> +
> +#ifndef __ASSEMBLY__
> +
> +
> +#define COMMAND_ONLY		0xFE
> +#define DATA_ONLY		0xFF

i dont see the point in the __ASSEMBLY__ protection.  this header isnt 
included by any header, and you dont actually define anything outside of the 
__ASSEMBLY__ which means including it from an assembly file doesnt make sense.

> +#define ACTIVE_LOW		0
> +#define ACTIVE_HIGH		1

considering you're already including spi.h, you might as well  re-use 
SPI_CS_HIGH instead of defining your own.

> +struct s5p_spi_platdata {
> +	struct s5p_gpio_bank *cs_bank;
> +	struct s5p_gpio_bank *clk_bank;
> +	struct s5p_gpio_bank *si_bank;
> +	struct s5p_gpio_bank *so_bank;

you need to include the header in this header which defines these structs.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100914/2e0a6e81/attachment.pgp