[U-Boot-Users] [PATCH v2] spi: Kill spi_chipsel table and introduce spi_setup()

[Mike Frysinger]

On Wednesday 06 February 2008, Haavard Skinnemoen wrote:
> On Tue, 5 Feb 2008 23:34:46 -0500
>
> Mike Frysinger <vapier at gentoo.org> wrote:
> > so the new SPI interface has this API:
> >  - void spi_init(void);
> >  - int spi_setup(int cs, unsigned int max_hz, unsigned int mode);
> >  - int spi_xfer(int cs, int bitlen, uchar *dout, uchar *din);
> >  - int spi_cs_is_valid(int cs);
> >  - void spi_cs_activate(int cs);
> >  - void spi_cs_deactivate(int cs);
>
> Yes, or at least that's the current API + my proposed patch.
>
> > there isnt a function to pair up with spi_setup() ?  for example, the
> > normal communication flow with a SPI flash:
> >  - spi_setup - turn on SPI
> >  - spi_cs_activate - assert CS
> >  - spi_xfer -
> > 	- op code (read/write/erase)
> > 	- address
> > 	- actual block data
> >  - spi_cs_deactivate - deassert CS
> >  - ??? - turn off SPI
>
> Right. I thought of spi_setup() more as a function that needs to be
> called one time per slave to set up communications parameters, not
> really for turning the SPI on as such.
>
> But perhaps it would make sense to combine those two functions. How
> about we turn it into
>
> /* 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);
>
> The claim/release naming also makes it clear that the SPI device driver
> has exclusive access to the bus between those two calls.

that makes sense to me ...

> Btw, the master driver is currently controlling the chip selects from
> spi_xfer(). I suspect we need to give clients greater control of the
> chip selects eventually.

i think dropping the cs parameter from spi_xfer() is the way to go since you 
should have claimed/asserted it already by the time you perform the 
transfer ?

> > also, what's the deal with spi_xfer() taking a length in # of bits ?  is
> > it realistic to transmit anything less tan 8 bits ?  the Linux kernel
> > driver does not support this, so it cant be a big need ...
>
> I don't know. That's unchanged from the original API. But I certainly
> wouldn't object if we turned it into a length in bytes.

since we're breaking things, might as well break this as well :)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080206/c984d1ee/attachment.pgp