On Fri, 22 Jun 2007 11:51:24 -0700, David Brownell <david-b@pacbell.net> wrote:
> > > This is a driver for SPI controller built into TXx9 MIPS SoCs.
> >
> > Looks mostly pretty good. I made a few minor changes/cleanups
> > in the appended version, notably:
> > - checking for spi->mode bits this code doesn't understand;
> > - updating to match latest patches;
> >
> > Note that if gpio_set_value() needs an mmiowb(), that seems like
> > a bug in this platform's GPIO code; other platforms don't require
> > I/O barriers after GPIO calls. Comments?
> >
> > Also:
> >
>
> ... yeah, -ENOPATCH, sorry. And the minor whitespace fixes.
>
> One more comment: surely platform_driver_probe() would be
> appropriate here?
Thank you for excellent review! I'll look at each comments surely
will update the driver but it may take a few days.
For now, I'm quite sure your patch is OK for me except for one thing:
> + * spi_tx99.c - TXx9 SPI controller driver.
Name it spi_txx9.c, please ;)
And for mmiowb() issue, I put it just only I was not sure whether
gpio_set_value() guarantee I/O barrier. Now I see i2c-gpio.c, etc. do
not have such barriers. I will remove these barriers and fix platform
gpio codes.
---
Atsushi Nemoto
|