Re: [spi-devel-general] Re: SPI

[David B. <da...@pa...>]

On Tuesday 29 November 2005 7:54 am, dmitry pervushin wrote:
> David,
> 
> some additional comments on your code:
> 
> - you've made spi_read_then_write DMA-capable... OK. But why you're
> using global for buffer ? 

To avoid a kmalloc()/kfree() overhead on each call, and keep this first
version of the framework simple and small.  And correct.


> You need to create the semaphore to protect 
> the global 'buf', so if you have two controllers on the same board, you
> would not perform two simultaneous transfers with these controllers,
> right ?

Yes, there IS a semaphore:

+int spi_write_then_read(...)
+{
+       static DECLARE_MUTEX(lock);
+       ...
+       down(&lock);
+       ...
+       up(&lock);
+       return status;
+}

If "one caller at a time" for that routine turns out to be a
problem, it's easily changed.  So far, it hasn't been one;
maybe it will be later.


> I's like to say that buf should be either controller data or 
> message data

The spi_message is allocated on the stack, which is inappropriate
for source/destination of a DMA buffer; that's not an option that
avoids kmalloc.

I'm not sure what you mean about "controller data".  It can't
be spi_device->controller_data since that's reserved for board
specific static state (e.g. chipselect callbacks, for boards
that use GPIOs instead of controller registers).



> - sometimes you're ignoring return codes of functions that return int,
> e.g., bus_register and class_register

Those two calls are in spi_init(), where they're unlikely to fail.

But you're right, the return codes SHOULD be used; feel free to
submit a patch.

- Dave