[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:
> 
> > slave = spi_setup_slave(host, dev, hz, 0 /*| SPI_ACCESS_EXCLUSIVE */);
> > if (!slave)
> > 	return;
> > /* Now you MUST release the slave / host */
> > spi_xfer();
> > ...
> > spi_release();
> 
> I think you should set up a slave exactly once before starting to
> communicate with it. Once you've done that, you just execute sequences
> of
> 
> spi_claim_bus(slave);
> spi_xfer(slave, ...);
> /* possibly more transfers */
> spi_release_bus(slave);
> 
> We could also define commonly-used "canned" sequences, like spi_w8r8()
> (write 8 bits, read 8 bits).

Ok, I see what you mean now. But then we need another function - an 
opposite to spi_setup, to free any allocated RAM, etc. I thought this was 
going to happen in release_bus, but after this explanation this doesn't 
seem to be the case.

So, just add an spi_free(), and, as a counterpart to it, an spi_init() 
might sound better than spi_setup:-)

> > And you get access to the bus if there are no exclusive users, and you get 
> > exclusive access, only if there are no users currently at all.
> 
> I don't see how having multiple active users on the bus at the same
> time can possibly make any sense...once you start a transfer, you
> better make sure nobody else is doing a transfer at the same time since
> you're using the same MOSI/MISO/SCK lines...

No, not "active." Multiple users having set up different slaves. But not 
communicating simultaneously:-)

> > Looks like most of this API implementation is hardware independent, and 
> > should go into an spi.c?
> 
> Not really...what "claiming" the bus really means is highly hardware
> dependent. And SPI slave setup is mostly about decoding
> hardware-independent parameters like SCK rate and mode bits into
> hardware register values. But any convenience wrappers like spi_w8r8()
> probably belongs somewhere hardware-independent.

I thought, like (pseudocode)

static struct spi_host busses[SPI_BUSSES];

struct spi_slave *spi_init()
{
	list_for_each_entry(slave, &busses[bus].slaves, list) {
		if (slave->device == device)
			return (struct spi_slave *)-EBUSY;
	}
	slave = malloc();
	slave->bus = bus;
	slave->device = device;
	ret = busses[bus].init(slave);
	if (ret) {
		free(slave);
		return (struct spi_slave *)ret;
	}
	return slave;
}

int spi_xfer()
{
	list_for_each_entry(ix, &busses[bus].slaves, list) {
		if (ix == slave)
			break;
	}
	if (ix != slave)
		return -EINVAL;

	if (slave->bus->busy)
		return -EBUSY;

	return slave->bus->xfer();
}

...and so on, which is all quite hardware-independent.

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