linux-mips
[Top] [All Lists]

Re: [spi-devel-general] [PATCH] TXx9 SPI controller driver

To: david-b@pacbell.net
Subject: Re: [spi-devel-general] [PATCH] TXx9 SPI controller driver
From: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Date: Sun, 24 Jun 2007 00:41:59 +0900 (JST)
Cc: spi-devel-general@lists.sourceforge.net, sshtylyov@ru.mvista.com, linux-mips@linux-mips.org, ralf@linux-mips.org, mlachwani@mvista.com
In-reply-to: <200706221151.24959.david-b@pacbell.net>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <20070622.232111.36926005.anemo@mba.ocn.ne.jp> <200706221103.19761.david-b@pacbell.net> <200706221151.24959.david-b@pacbell.net>
Sender: linux-mips-bounce@linux-mips.org
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

<Prev in Thread] Current Thread [Next in Thread>