Guennadi Liakhovetski <[EMAIL PROTECTED]> wrote:
> On Thu, 8 May 2008, Haavard Skinnemoen wrote:
> > Therefore, I'm going to remove it in the next version of my patchset.
> > If you can tell me how it's supposed to work, I can try to minimize the
> > breakage.
>
> Would be better if we could avoid any breakage completely, please.
That's certainly what I'm aiming for, but I need someone to help me
test it.
> I added this function, because 1) the current spi_xfer() doesn't support
> multiple SPI busses, and 2) is not particularly friendly when the SPI
> controller itself controls chipselects. As far as I can see your new
> proposed API doesn't solve the former of these problems either. One could
> get around this problem by numbering all chipselects on all busses
> through, but that would be too ugly.
I don't think global chip select numbering would be _that_ ugly, but
yeah, maybe we should add a bus parameter as well.
> So, the spi_select does just that -
> selects a bus and a device to talk to. Of course this is racy, but as long
> as there's no multitasking, it should be ok.
Yeah, multitasking isn't an issue.
> As you certainly noticed while working on your API improvements, the
> current API is very unflexible. But as I didn't have resources to rework
> it completely and change multiple existing drivers, I chose the lesser
> evil - added an auxiliary function.
Lesser evil compared to what exactly? Than participating in the
discussion about the new API?
> Wouldn't an API like
>
> struct spi_slave *spi_attach(int bus, int cs); /* also does init */
> int spi_xfer(struct spi_slave *slave, bitlen, dout, din);
> void spi_detach(struct spi_slave *slave);
>
> (approximately) be better?
I did propose something similar a while ago:
/* Set slave-specific parameters and enable SPI */
int spi_claim_bus(int cs, unsigned int max_hz, unsigned int mode);
/* Disable SPI */
void spi_release_bus(int cs);
But I have to say I like the idea about passing a spi_slave
"handle" around...
How about something like this?
/*
* Set up communication parameters for a SPI slave. This doesn't
* necessarily enable the controller.
*/
struct spi_slave *spi_create_slave(int bus, int cs, unsigned int
max_hz, unsigned int mode);
/*
* Get exclusive access to the SPI bus for communicating with the given
* slave. Returns a negative value if the bus is busy (drivers may omit
* such checks if they don't want the extra code/data overhead.)
*/
int spi_claim_bus(struct spi_slave *slave);
/*
* Release the SPI bus. This may disable the controller and/or put it
* in low-power mode
*/
void spi_release_bus(struct spi_slave *slave);
/*
* Transfer data on the SPI bus.
*
* flags can be a combination of any of the following bits:
* SPI_XFER_BEGIN: Assert CS before starting the transfer
* SPI_XFER_END: Deassert CS after the transfer
*/
int spi_xfer(struct spi_slave *slave, int bitlen, const void *dout,
void *din, unsigned long flags);
What do you think?
Haavard
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users
