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

[Sudhakar Rajashekhara]

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