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

[Nick Thompson]

On 23/12/09 07:44, Sudhakar Rajashekhara wrote:
> From: Sekhar Nori <nsekhar at ti.com>
> 
> This adds a driver for the SPI controller found on davinci
> based SoCs from Texas Instruments.
> 
> Signed-off-by: Sekhar Nori <nsekhar at ti.com>
> Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj at ti.com>
> ---
>  drivers/spi/Makefile      |    1 +
>  drivers/spi/davinci_spi.c |  205 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/spi/davinci_spi.h |   84 ++++++++++++++++++
>  3 files changed, 290 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/spi/davinci_spi.c
>  create mode 100644 drivers/spi/davinci_spi.h

...

> diff --git a/drivers/spi/davinci_spi.h b/drivers/spi/davinci_spi.h
> new file mode 100644
> index 0000000..b3bf916
> --- /dev/null
> +++ b/drivers/spi/davinci_spi.h
> @@ -0,0 +1,84 @@
> +/*
> + * Register definitions for the DaVinci SPI Controller
> + */
> +
> +/* Register offsets */
> +#define DAVINCI_SPI_GCR0	0x0000
> +#define DAVINCI_SPI_GCR1	0x0004
> +#define DAVINCI_SPI_INT0	0x0008
> +#define DAVINCI_SPI_LVL		0x000c
> +#define DAVINCI_SPI_FLG		0x0010
> +#define DAVINCI_SPI_PC0		0x0014
> +#define DAVINCI_SPI_PC1		0x0018
> +#define DAVINCI_SPI_PC2		0x001c
> +#define DAVINCI_SPI_PC3		0x0020
> +#define DAVINCI_SPI_PC4		0x0024
> +#define DAVINCI_SPI_PC5		0x0028
> +#define DAVINCI_SPI_DAT0	0x0038
> +#define DAVINCI_SPI_DAT1	0x003c
> +#define DAVINCI_SPI_BUF		0x0040
> +#define DAVINCI_SPI_EMU		0x0044
> +#define DAVINCI_SPI_DELAY	0x0048
> +#define DAVINCI_SPI_DEF		0x004c
> +#define DAVINCI_SPI_FMT0	0x0050
> +#define DAVINCI_SPI_FMT1	0x0054
> +#define DAVINCI_SPI_FMT2	0x0058
> +#define DAVINCI_SPI_FMT3	0x005c
> +#define DAVINCI_SPI_INTVEC0	0x0060
> +#define DAVINCI_SPI_INTVEC1	0x0064

I think this ought to be a C structure, rather than register offsets?

...

> +
> +struct davinci_spi_slave {
> +	struct spi_slave slave;
> +	void		*regs;

This should have the type of the C structure to be defined above.

> +	u32		mr;
> +	unsigned int freq;
> +};
> +

...

> +
> +#define spi_readl(ds, reg)					\
> +	readl(ds->regs + DAVINCI_SPI_##reg)
> +#define spi_writel(ds, reg, value)				\
> +	writel(value, ds->regs + DAVINCI_SPI_##reg)

These can be rewritten (and the usages changed slightly) to access via the structure.
You will then not be circumventing any type checking.

Maybe these defs then become too trivial and can be dropped altogether?

Regards,
Nick.