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

Sudhakar Rajashekhara
2010-01-05T13:29:05Z

On Tue, Jan 05, 2010 at 15:22:39, Nick Thompson wrote:
> 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?
>

My mistake. I'll correct it.

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

I'll remove such things and re-submit the patch.

Regards,
Sudhakar