[U-Boot-Users] [PATCH v2 1/3] New i.MX31 SPI driver

[Guennadi Liakhovetski]

On Thu, 8 May 2008, Haavard Skinnemoen wrote:

> Guennadi Liakhovetski <lg at denx.de> wrote:
> > > Oh great. We can do API changes without even mentioning it in the
> > > change log now?  
> > 
> > Right, sorry, should have mentioned it. Although, the API change is one 
> > added function spi_select(), which you do not have to implement. So, I 
> > don't think I have broken anything.
> 
> The problem is that it is completely undocumented, and it appears to
> have a lot of overlap with the spi_setup() function I proposed several
> times a while back.
> 
> Besides, it's only optional up to the point where drivers start using
> it...and the mc13783-rtc driver does appear to be using it, so it isn't
> really optional anymore. And I suspect lots of other drivers really
> _do_ need this kind of thing, which is why I proposed the spi_setup()
> interface to begin with. You can always get around this by adding
> tweaks to the board/cpu/driver code for your particular setup, but I
> think very few drivers work as expected out of the box on a new board
> or platform. So I don't think an interface like this _should_ be
> optional.
> 
> 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.

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. 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.

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.

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?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de