Re: [PATCH/RFC] SPI: async message handing library update

From: Vitaly Wool
Date: Thu Dec 22 2005 - 17:09:18 EST


David Brownell wrote:

if (!spi->max_speed_hz)
spi->max_speed_hz = 500 * 1000;

/* nsecs = max(50, (clock period)/2), be optimistic */
cs->nsecs = (1000000000/2) / (spi->max_speed_hz);
if (cs->nsecs < 50)
cs->nsecs = 50;




Suggest not to hardcode values here.



I suppose it'd make sense to just fail if max_speed_hz is invalid,
and if there's some board that an bitbang at over 10MHz we should
avoid getting in its way.


Well, why do this?

/* protocol tweaks before next transfer */
if (t->delay_usecs)
udelay(t->delay_usecs);




Suggest nsecs here as well.



The relevant chip delays seem to be specified in usecs ... I don't
like using nsecs for the clock timings, but without doing that it'd
be impractical to define rates at the levels the hardware actually
uses. There are still some "nsec" leakages out of the real-bitbang
code up to the next level, fixable over time.



Ok.




/* FIXME if bitbang->use_dma, dma_map_single()
* before the transfer, and dma_unmap_single()
* afterwards, for either or both buffers...
*/

please *please* *_please_*!!! don't do it! :)
Let the controller driver do it *only in case it's not working in PIO!*



OK. That'd be more work for the controller driver, but you're
right that a lot of the drivers using these utilities are rather
likely to be PIO-oriented. If they want DMA speedups, they can
do the mappings themselves (in cases where the driver didn't
do them already).


Agreed :)




Another one: I just feel comfortabel with using 'bitbang' term for the variety of SPI stuff which this library suits.



You _do_ feel comfortable with it? I actually feel a bit odd, since
only one of the three driver types is really bitbanging. And in fact
it still bothers me that the other two tie down a task, but that's
the price for reusing common infrastructure.


Oh sorry, of course I meant "I just don't feel comfortable..."

BTW: the message handling is one per-transfer basis for bitbang. But in this case it's not possible to imlement chained DMA transfers (2 channels, one for Rx, one for Tx, basically that's your sample use case :)

Vitaly
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/