> -----Original Message----- > From: Kevin Hilman [mailto:[email protected]] > Sent: Tuesday, June 02, 2009 5:37 PM > To: Paulraj, Sandeep > Cc: [email protected] > Subject: Re: [PATCH] DaVinci SPI Support > > [email protected] writes: > > > Patch adds support for DaVinci SPI driver. This was tested on > > the DM355 EVM using the EEPROM present on the EVM. The SPI EEPROM driver > > for ATMEL(at25) EEPROM chips was used as the client driver. > > > > Signed-off-by: Sandeep Paulraj <[email protected]> > > Dave is the SPI expert (and maintainer) so I'll defer to him here, but > here's my $0.02. > > I'd rather see the shared platform_data struct (struct > davinci_spi_platform_data, and it's related enum, CS etc.) defined in > arch code (mach/spi.h) which is then included from > linux/spi/davinci_spi.h. [Sandeep] OK. That's not a big change > > Aside from it being a cleaner separation IMHO, it also makes the > upstream submission easier since the platform code can go upstream > first without having a build-time dependency on the driver. [Sandeep] OK > > Also minor nit below... > > [...] > > > # SPI protocol drivers (device/link on bus) > > diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c > > new file mode 100644 > > index 0000000..173439d > > --- /dev/null > > +++ b/drivers/spi/davinci_spi.c > > +static int davinci_spi_bufs_prep(struct spi_device *spi, > > + struct davinci_spi *davinci_spi, > > + struct davinci_spi_config *spi_cfg) > > +{ > > + u32 sPIPC0; [Sandeep] I'll take acre of it in the next set > > Why the mixed-case name? Just use spipco, or better yet something > more human readable like pin_control. [Sandeep] Any comments/concerns on the driver itself or shall we wait for Dave? The driver itself seems to work! > > Kevin
_______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
