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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux