[U-Boot] [PATCH 10/34] spi: Add zynq qspi controller driver

Jagan Teki
2013-11-06T06:11:02Z

Hi Thomas,

On Wednesday 06 November 2013 01:29 AM, thomas.langer at lantiq.com wrote:
> Hello Jagan,
>
> I have some comments and questions:
>
> Am 05.11.2013 18:51, schrieb Jagannadha Sutradharudu Teki:
>> +
>> +/* Definitions of the flash commands - Flash insts in ascending order */
>> +#define ZYNQ_QSPI_FLASH_INST_WRSR 0x01 /* Write status register */
>> +#define ZYNQ_QSPI_FLASH_INST_PP 0x02 /* Page program */
>> +#define ZYNQ_QSPI_FLASH_INST_WRDS 0x04 /* Write disable */
>> +#define ZYNQ_QSPI_FLASH_INST_RDSR1 0x05 /* Read status register 1 */
>> +#define ZYNQ_QSPI_FLASH_INST_WREN 0x06 /* Write enable */
>> +#define ZYNQ_QSPI_FLASH_INST_AFR 0x0B /* Fast read data bytes */
>> +#define ZYNQ_QSPI_FLASH_INST_BE_4K 0x20 /* Erase 4KiB block */
>> +#define ZYNQ_QSPI_FLASH_INST_RDSR2 0x35 /* Read status register 2 */
>> +#define ZYNQ_QSPI_FLASH_INST_BE_32K 0x52 /* Erase 32KiB block */
>> +#define ZYNQ_QSPI_FLASH_INST_RDID 0x9F /* Read JEDEC ID */
>> +#define ZYNQ_QSPI_FLASH_INST_SE 0xD8 /* Sector erase (usually 64KB)*/
>> +
> Why needs the spi controller this list of flash commands?
> It is the job of the flash driver to handle this, the spi controller
> only forwards this to the devices!
>
>> +
>> +/* List of all the QSPI instructions and its format */
>> +static struct zynq_qspi_inst_format flash_inst[] = {
>> + {ZYNQ_QSPI_FLASH_INST_WRSR, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
>> + {ZYNQ_QSPI_FLASH_INST_PP, 4, ZYNQ_QSPI_TXD_00_00_OFFSET},
>> + {ZYNQ_QSPI_FLASH_INST_WRDS, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
>> + {ZYNQ_QSPI_FLASH_INST_RDSR1, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
>> + {ZYNQ_QSPI_FLASH_INST_WREN, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
>> + {ZYNQ_QSPI_FLASH_INST_AFR, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
>> + {ZYNQ_QSPI_FLASH_INST_BE_4K, 4, ZYNQ_QSPI_TXD_00_00_OFFSET},
>> + {ZYNQ_QSPI_FLASH_INST_RDSR2, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
>> + {ZYNQ_QSPI_FLASH_INST_BE_32K, 4, ZYNQ_QSPI_TXD_00_00_OFFSET},
>> + {ZYNQ_QSPI_FLASH_INST_RDID, 1, ZYNQ_QSPI_TXD_00_01_OFFSET},
>> + {ZYNQ_QSPI_FLASH_INST_SE, 4, ZYNQ_QSPI_TXD_00_00_OFFSET},
>> + /* Add all the instructions supported by the flash device */
>> +};
> Is this table used to encode which parts needs DUAL or QUAD transfers?
> And are you sure the table is complete? The flash drivers don't use more
> instructions?
> What happens if some device is connected, which is no flash?
>> +
>> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
>> + void *din, unsigned long flags)
>> +{
>> + struct zynq_qspi_slave *zslave = to_zynq_qspi_slave(slave);
>> + u32 len = bitlen / 8, tx_tvl;
>> + u32 buf, status;
>> +
>> + debug("spi_xfer: bus:%i cs:%i bitlen:%i len:%i flags:%lx\n",
>> + slave->bus, slave->cs, bitlen, len, flags);
>> +
>> + if (bitlen == 0)
>> + return -1;
>> +
>> + if (bitlen % 8) {
>> + debug("spi_xfer: Non byte aligned SPI transfer\n");
>> + return -1;
>> + }
>> +
>> + if (flags & SPI_XFER_BEGIN)
>> + spi_cs_activate(slave);
>> +
>> + zslave->tx_len = len;
>> + zslave->rx_len = len;
>> + zslave->tx_buf = dout;
>> + zslave->rx_buf = din;
>> + while (zslave->rx_len > 0) {
>> + /* Write the data into TX FIFO - tx threshold is fifo_depth */
>> + tx_tvl = 0;
>> + while ((tx_tvl < zslave->fifo_depth) && zslave->tx_len) {
>> + if (zynq_qspi_process_tx(zslave) < 0) {
>> + flags |= SPI_XFER_END;
>> + goto out;
>> + }
>> + tx_tvl++;
>> + }
>> +
>> + /* Check TX FIFO completion */
>> + if (zynq_qspi_check_txfifo(zslave) < 0) {
>> + flags |= SPI_XFER_END;
>> + goto out;
>> + }
>> +
>> + /* Read the data from RX FIFO */
>> + status = readl(&zslave->base->isr);
>> + while (status & ZYNQ_QSPI_IXR_RXNEMPTY_MASK) {
>> + buf = readl(&zslave->base->rxdr);
>> + if (zslave->rx_len < 4)
>> + zynq_qspi_read(zslave, buf, zslave->rx_len);
>> + else
>> + zynq_qspi_read(zslave, buf, 4);
>> + status = readl(&zslave->base->isr);
>> + }
>> + }
>> +
>> +out:
>> + if (flags & SPI_XFER_END)
>> + spi_cs_deactivate(slave);
>> +
>> + return 0;
>> +}
> In this function I miss the parts, where the caller (e.g. the flash
> driver) tells the controller to transfer some parts in DUAL or QUAD mode!

Yes- I sent this as part of zynq support series.
I will fix the details where controller won't need to aware of flash
specific data in coming versions.

--
Thanks,
Jagan.