On Thursday 05 February 2009, Brian G Rhodes wrote:
> I appreciate the review.
> 
> David Brownell wrote:
> > I don't think I see any point to the IRQ support, or
> >   
> There is no need presently for the interrupt handler, however I think 
> using it for a completion to notify the read/duplex routines would 
> provide better support for SPI data transfer.  I am currently only using 
> SPI for control.  Is there a benefit over the polling of RXINTFLAG I am 
> currently using?  If not, I can just drop it.

ISTR there's a dm355 erratum explaining some issue with polling;
as in, don't poll the most obvious register.

It's general goodness not to request resources you don't really
need.  Example, for dm355 the spi1 IRQs are muxed against some
of the EDMA IRQs, so spi1-0 in particular should not be used.
Also spi1-1 and spi2-0 are muxed against EDMA TC error IRQs,
which may someday be needed.


> > the empty cleanup() method.
> >
> > Don't disable the controller during setup(); there may
> > be an ongoing transfer on a different chipselect.
>
> I am a bit confused how there could be another transfer active.  Can you 
> explain this?

While the workqueue is crunching through a queue of messages
for one spi_device, another task could issue spi_setup() for
a different device on the same controller.

The workqueue should just be taking the device parameters
that have already been set up, and working with them.  It
may need to override the bitwidth or transfer speed on a
per-transfer basis, but at that point it can rely on the
fact that no other task is allowed to issue spi_setup for
that device (since it has outstanding messages).

- Dave

p.s. I don't think I noticed anything calculating the
     divisor to use from the bit rate passed into setup()...


_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to