All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Jander <david.jander@protonic.nl>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5
Date: Fri, 20 Aug 2010 14:15:57 +0200	[thread overview]
Message-ID: <201008201415.57781.david.jander@protonic.nl> (raw)
In-Reply-To: <4C6E64C0.3080902@denx.de>


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.

  reply	other threads:[~2010-08-20 12:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-20  8:20 [U-Boot] [PATCH] Use common function to set GPIOs for MX3 and MX5 Stefano Babic
2010-08-20  8:46 ` David Jander
2010-08-20  9:29 ` David Jander
2010-08-20 10:01   ` Stefano Babic
2010-08-20 10:07     ` David Jander
2010-08-20 10:20       ` Stefano Babic
2010-08-20 10:30         ` David Jander
2010-08-20 11:19           ` Stefano Babic
2010-08-20 12:15             ` David Jander [this message]
2010-08-20 13:35               ` Stefano Babic
2010-08-23  8:50                 ` David Jander
2010-08-23  9:14                   ` David Jander
2010-08-23 10:37                     ` Stefano Babic
2010-08-23 11:30                       ` David Jander
2010-08-23 15:55                         ` Stefano Babic
2010-08-23 17:18                           ` Stefan Roese
2010-08-23 21:03                             ` Detlev Zundel
2010-08-23 21:53                               ` Mike Frysinger
2010-08-26  9:09                                 ` Detlev Zundel
2010-08-23 10:28                   ` Stefano Babic
2010-08-20 10:55 ` [U-Boot] [PATCH V2] " Stefano Babic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201008201415.57781.david.jander@protonic.nl \
    --to=david.jander@protonic.nl \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.