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

[Vitaly Wool <vwool@ru.mvista.com>]

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/