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

[Jagan Teki]

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.