[U-Boot] [PATCH 3/6] spi: Add designware master SPI DM driver used on SoCFPGA

[Pavel Machek <pavel@denx.de>]

Hi!

You tripped my spell-checker, sorry.

> This patch adds the driver for the Designware master SPI controller. This
> IP core is integrated on the Altera SoCFPGA. This implementation is a
> driver model (DM) implementation. So multiple SPI drivers can be used.
> Thats necessary, since SoCFPGA also integrates the Cadence QSPI controller
> used to connect the SPI NOR flashes. Without DM, using multiple SPI
> driver is not possible.

drivers.

> This driver is very loosly based on the Linux driver. Most of the

loosely?

> driver is removed. Only the polling loop for the transfer is really used
> from this driver. As we don't support interrupts and DMA right now.

, as


> + * Very loosly based on the Linux driver version which is:

loosely. (And citing filename here might be useful.)
> +
> +/* Register offsets */
> +#define DW_SPI_CTRL0			0x00
> +#define DW_SPI_CTRL1			0x04
> +#define DW_SPI_SSIENR			0x08
> +#define DW_SPI_MWCR			0x0c
> +#define DW_SPI_SER			0x10
> +#define DW_SPI_BAUDR			0x14
> +#define DW_SPI_TXFLTR			0x18
> +#define DW_SPI_RXFLTR			0x1c
> +#define DW_SPI_TXFLR			0x20
> +#define DW_SPI_RXFLR			0x24
> +#define DW_SPI_SR			0x28
> +#define DW_SPI_IMR			0x2c
> +#define DW_SPI_ISR			0x30
> +#define DW_SPI_RISR			0x34
> +#define DW_SPI_TXOICR			0x38
> +#define DW_SPI_RXOICR			0x3c
> +#define DW_SPI_RXUICR			0x40
> +#define DW_SPI_MSTICR			0x44
> +#define DW_SPI_ICR			0x48
> +#define DW_SPI_DMACR			0x4c
> +#define DW_SPI_DMATDLR			0x50
> +#define DW_SPI_DMARDLR			0x54
> +#define DW_SPI_IDR			0x58
> +#define DW_SPI_VERSION			0x5c
> +#define DW_SPI_DR			0x60

Any chance to convert this to structure?

> +#define RX_TIMEOUT			1000

comment with units would be welcome.
 
> +
> +struct dw_spi_platdata {
> +	s32 frequency;		/* Default clock frequency, -1 for none */
> +	void __iomem *regs;
> +};
> +
> +struct dw_spi_priv {
> +	void __iomem *regs;
> +	unsigned int freq;		/* Default frequency */
> +	unsigned int mode;
> +
> +	int bits_per_word;
> +	u8 cs;			/* chip select pin */
> +	u8 n_bytes;		/* current is a 1/2/4 byte op */
> +	u8 tmode;		/* TR/TO/RO/EEPROM */
> +	u8 type;		/* SPI/SSP/MicroWire */
> +	int len;

Having both n_bytes and len is "interesting". Turn n_bytes into
"word_size"?

> +static int dw_spi_probe(struct udevice *bus)
> +{
> +	struct dw_spi_platdata *plat = dev_get_platdata(bus);
> +	struct dw_spi_priv *priv = dev_get_priv(bus);
> +
> +	priv->regs = plat->regs;
> +	priv->freq = plat->frequency;
> +
> +	/* Currently only bits_per_word == 8 supported */
> +	priv->bits_per_word = 8;
> +	priv->n_bytes = 1;

n_bytes -> bytes_per_word? Why we have two variables for same
information?

> +	/*
> +	 * Another concern is about the tx/rx mismatch, we
> +	 * though to use (priv->fifo_len - rxflr - txflr) as

"thought about using"?


> +	while (max--) {
> +		rxw = dw_readw(priv, DW_SPI_DR);
> +		debug("%s: rx=0x%02x\n", __func__, rxw);
> +		/* Care rx only if the transfer's original "rx" is not null */

"Care about"?
> +
> +	/* spi core configured to do 8 bit transfers */

"core is"?

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html