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

Pavel Machek
2014-11-12T17:51:42Z

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