[U-Boot] [PATCH v2] TI DaVinci: Driver for the davinci SPI controller

[Nick Thompson <nick.thompson@ge.com>]

On 05/01/10 04:47, Sudhakar Rajashekhara wrote:
> From: Sekhar Nori <nsekhar@ti.com>
> 
> This adds a driver for the SPI controller found on davinci
> based SoCs from Texas Instruments.
> 
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> ---
> From the previous version following have been modified:
>  1. Sorted the entries in drivers/spi/Makefile alphabetically.
>  2. Implemented dummy functions for spi_cs_is_valid(),
>     spi_cs_activate() and spi_cs_deactivate().
>  3. Added GPL header for drivers/spi/davinci_spi.h file.
>  4. Added protection against multiple inclusion of SPI header
>     file.
>  5. Replaced the macro based register offsets in SPI header
>     file with structure.
>  6. Replaced the spi_readl and spi_writel functions with
>     readl and writel respectively.
> 
>  drivers/spi/Makefile       |    1 +
>  drivers/spi/davinci_spi.c  |  221 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/spi/davinci_spi.h  |  102 ++++++++++++++++++++
>  include/configs/da830evm.h |    2 +-

No sign of this file in the patch set. Is this intentional?

>  4 files changed, 325 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/spi/davinci_spi.c
>  create mode 100644 drivers/spi/davinci_spi.h

...

> diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c
> new file mode 100644
> index 0000000..c3f1810
> --- /dev/null
> +++ b/drivers/spi/davinci_spi.c
> @@ -0,0 +1,221 @@

...

> +
> +	/* CS, CLK, SIMO and SOMI are functional pins */
> +	writel((SPIPC0_EN0FUN_MASK) | (SPIPC0_CLKFUN_MASK) |
> +		(SPIPC0_DOFUN_MASK) | (SPIPC0_DIFUN_MASK), &ds->regs->pc0);

There seem to be numerous cases, here and elsewhere in the file where bare defines
are referenced within parenthesis for no obvious reason. If they are needed they
should be in the #define statement. I think in all cases they are, so the above
lines should be something like...

	/* CS, CLK, SIMO and SOMI are functional pins */
	writel((SPIPC0_EN0FUN_MASK | SPIPC0_CLKFUN_MASK |
		SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0);

...which also corrects the alignment of the last line.

Regards,
Nick.