On 23/12/09 07:44, Sudhakar Rajashekhara wrote:
> From: Sekhar Nori <nsek...@ti.com>
> 
> This adds a driver for the SPI controller found on davinci
> based SoCs from Texas Instruments.
> 
> Signed-off-by: Sekhar Nori <nsek...@ti.com>
> Signed-off-by: Sudhakar Rajashekhara <sudhakar....@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.


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to