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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to