[U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5

[David Jander]

Hi Stefano,

On Friday 20 August 2010 01:19:28 pm Stefano Babic wrote:
> > Will do.
> > Btw, do you have any idea why spi_xchg_single() hangs while transmitting
> > the second word without claiming the bus again?
> 
> Can you see where does it hang ? Which device is connected to your SPI
> bus ? Does it work with the pmic (I think so, see later...).

Just figured out one big mistake. I was debugging spi_flash.c, and had 
CONFIG_ENV_IS_IN_SPI_FLASH set. That means, first SPI access is done before 
malloc is available, and guess what? spi_setup_slave() uses malloc ;-)
So I did something in the way of this:

static struct mxc_spi_slave mxc_spi_slave_pool[4];

struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
			unsigned int max_hz, unsigned int mode)
{
	unsigned int ctrl_reg;
	struct mxc_spi_slave *mxcs;
	int ret;

	if (bus >= ARRAY_SIZE(spi_bases))
		return NULL;

	//mxcs = malloc(sizeof(struct mxc_spi_slave));
	//if (!mxcs)
	//	return NULL;
	mxcs = &mxc_spi_slave_pool[cs];

The C++ comments show original code, the line below is new.

It still hangs though, waiting for MXC_CSPICTRL_TC to become active. Busy 
finding out why...

I see a one byte access followed by a 5 byte access, of which only 8.5 clock 
pulses come out of SCLK, and then nothing more. MXC_CSPICTRL_TC stays low.

> > Also, I don't know if you already fixed mxc_spi.c, to use the correct
> > byte- ordering when sending u8 buffers.
> 
> Well, it seems we are working at the same issues, and probably it is
> better if we try some coordination ;-)

Good idea.

> I have already fix the byte ordering, but I am fixing now the misaligned
> access (this is the reason I have not sent the changes for gpios in the
> mxc_spi.c: I am reworking this driver...). In the mainline driver, only
> 32-bit aligned buffers are allowed, and this is a strong limitation. I
> cannot use some other components in u-boot, such as SPI flash, because
> the code allocates u8 buffers that can be disaligned. And with other
> devices (sensors, eeprom, ...) does not work, because most of them
> require to transfer only one or two bytes as command.

I assumed that u-boot spi drivers use only u8 buffers for simplicity.
So my fix looks as this:

int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
		void *din, unsigned long flags)
{
	int n_blks = (bitlen + 31) / 32;
	u32 out, in;
	u8 *in_b, *out_b;
	int i;
	
	out_b = (u8 *)dout;
	in_b = (u8 *)din;
	
	for(i = 0; i< n_blks; i++, bitlen -= 32) {
		if(dout) {
			out = out_b[0];
			if(bitlen > 8)
				out = (out<<8) | out_b[1];
			if(bitlen > 16)
				out = (out<<8) | out_b[2];
			if(bitlen > 24)
				out = (out<<8) | out_b[3];
			out_b += 4;
		} else {
			out=0;
		}
		in = spi_xchg_single(slave, out, bitlen, flags);
		if(din) {
			if(bitlen > 24) {
				in_b[3] = in & 0xff;
				in >>= 8;
			}
			if(bitlen > 16) {
				in_b[2] = in & 0xff;
				in >>= 8;
			}
			if(bitlen > 8) {
				in_b[1] = in & 0xff;
				in >>= 8;
			}
			in_b[0] = in & 0xff;
			in_b += 4;
		}
	}
	return 0;
}

Does this sound like it could work?

> Now that I think...is it maybe your case now ? The FIFO can be accessed
> only as word, other accesses are not allowed according to the manual.

I am aware of that. That's why I used spi_xchg_single() as is.

> However, I am currently working on several issues for MX51. It should be
> nice to know which are your plans to save both some time ;-)

Well, I am in a bit of a hurry, and essentially what I need is to be able to 
access SPI-nor flash (spansion type) for environment and booting linux.
MMC/SD access would be nice, but is not yet necessary.

> > I have a fix, but it is not yet ready.
> > I essentially renamed spi_xfer() to spi_xfer_fsl(), to be used in the
> > (broken) pmic driver,
> 
> As I said, I changed the pmic driver, too. I do not agree we must have
> "special" functions, only because something is broken. The pmic works
> because it is connected to the FSL SPI controller, and the endianess is
> consistent. However, it is common for a SPI driver to allocate a "char"
> buffer, and the first byte in the buffer is the first byte sent to the
> SPI bus. This is not the case with mxc_spi.c

I know. I thought to do it in two steps: Fix mxc_spi.c with a workaround for 
the pmic driver (which amounts to '#define spi_xfer spi_xfer_fsl' at the 
beginning of this driver basically) and fix the pmic driver later, since it is 
probably not trivial, and needs to be done carefully (you know, one can smoke 
a board by mistake :-)

Best regards,

-- 
David Jander
Protonic Holland.