lkml.org 
[lkml]   [2008]   [Dec]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [patch 2.6.28-rc9] spi: spi_write_then_read() regression fix
Date
On Monday 22 December 2008, Ingo Oeser wrote:
> Hi David,
>
> On Monday 22 December 2008, you wrote:
> > The write_then_read() helper is
> > simplifying a common half-duplex idiom for short operations,
> > but the hardware still does full duplex. Buffer layout is:
> >
> > Before: WWWWW0000000
> > After: xxxxxRRRRRRR
> >
> > That is, for every bit shifted out (W, 0) another one gets
> > shifted in (x, R). The I/O primitive essentially swaps
> > contents of a one-word shift register between master and
> > slave; or, sequences of such words. Words don't need to
> > be byte-size, though that's a common option.
>
> ...
>
> Wow, what interesting hardware logic and a nice explanation.

It's standard SPI ... yes, it's kind of interesting, and I've
not come across anything else that synchronizes RX and TX in
quite the same way.

I suspect that hardware designers find the ability to get
quick transfers (tens of megabits/second) from just a shift
register is fairly attractive. Less work than high speed
I2C, for one example.


> Could you put that into a comment somewhere close to those helpers?

Fair enough. That file does lack any mention of the I/O model
for SPI ... and it's certainly a bit of a surprise to anyone
who's not had to look at it before!


> You can safely assume, that any code which Linus doesn't understand
> is non-trivial and needs a comment :-)

I'm not sure I'd agree with that -- while he's many things,
"expert in everything" is not one of them! -- but I do agree
that comment would be worth adding.


However, with regards to $PATCH ... considering that the '28
release is just around the corner, I changed my mind and would
suggest just reverting f9b90e39cbc5c4d6ef60022fd1f25d541df0aad1
much though I very much like that fix in general.

Reason being: it needs a bit more work, I forgot that it will
break for example drivers using drivers/spi/omap_uwire.c since
that is built on top of truly half-duplex hardware. (Used with
OMAP1 SOCs.) The kerneldoc says it's for providing MicroWire
style transactions, so it's kind of significant that it work
on MicroWire hardware too. ;)

So an updated version would need to both zero the TX bits when
using normal full-duplex SPI hardware ... and use more or less
the current code when running on half-duplex variants.

- Dave




\
 
 \ /
  Last update: 2008-12-23 19:41    [W:0.035 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site