[spi-devel-general] Re: [PATCH/RFC] SPI: async message handing library update

[Vitaly W. <vw...@ru...>]

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