"Paulraj, Sandeep" <[email protected]> writes:

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

No specific comments on the driver myself.

Kevin


_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to