Re: [PATCH] spi: add support for SPI over SuperH SCI pins

[Paul Mundt <lethal@linux-sh.org>]

On Mon, Jan 21, 2008 at 08:22:22PM -0800, David Brownell wrote:
> On Monday 21 January 2008, Paul Mundt wrote:
> 
> > > > +#define spidelay(x) ndelay(x)
> > > > +
> > > > +#define	EXPAND_BITBANG_TXRX
> > > > +#include <linux/spi/spi_bitbang.h>
> > > > +
> > 
> > This is rather unorthodox..
> 
> return -ENOPATCH;)
> 
> I've seen similar idioms used for years.  It's not dissimilar
> to "#ifdef __KERNEL__" except for the code audiences:  two
> different drivers, vs two different address spaces.
> 
> Similar approaches have been used to expand bit manipulation
> algorithms in other contexts ... like some X11 servers I once
> had to cope with.  In fact that was infamous for breaking CPP
> on new platforms; that was before C grew inlines!  But the
> rationale is still the same:  make sure the compiler has every
> opportunity to optimize each variant of those inner loops,
> since they're performance-critical.
> 
> And it works.  I append the code generated on one ARM which
> inlines the bitops ... the inner loop isn't as fast as one
> could code by hand, but if you consider it's four GPIO ops
> then it's not bad.  And it's twice as fast as going through
> a subroutine call for each bitop.  That's easily visible on
> block I/O paths that unfortunately can't use hardware SPI
> on that system.

On Mon, Jan 21, 2008 at 09:26:32PM -0800, David Brownell wrote:
> Minor clarification:  twice as fast for loads involving lots
> of medium-size buffers (1K+).  Down from one minute(!) to put
> new kernels to the only place that they can boot from ... it's
> the usual case where a 5x speedup at one place is very visible,
> but doesn't add up to 5x overall; other issues start to show
> up more (like unchanged per-byte overheads).
> 

The -ENOPATCH thing was intentional, I was more fishing for an
explanation of why this magical sequence existed than lobbying for its
replacement. drivers/spi/ isn't in danger of looking like crypto/ in the
name of code generation just yet, at least. ;-)

Thanks for the clarification.