[U-Boot] [PATCH 1/3] spi: Add zynq qspi controller driver

[Jagan Teki]

Hi,

On Sun, Aug 11, 2013 at 3:42 PM,  <thomas.langer at lantiq.com> wrote:
> Hi Jagan,
>
>> +
>> +/* 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)*/
>
> As I have written some time ago, I think it is a bad idea to parse the
> flash commands in the spi driver!
> The flash driver, who wants to write these commands and should know the details,
> which parts of the message needs to be send as dual/quad transfer, should
> provide this information to the spi driver (probably as flags for spi_xfer())
>
> A first idea for the flags:
> - the X refers to different numbers, as required by the flash (depending on the
> addressing mode (3/4 byte) or if the address is already sent as DUAL/QUAD)
> SPI_XFER_X_DUAL
> SPI_XFER_X_QUAD
>
> So having 0-5 for X should be enough to support all flashes
> -> 0 if the command is already in DUAL/QUAD mode
> -> 5 for command + 4byte address
> (I am not sure about dummy bytes on read)
>
> So for example:
> switch to dual transfer after first byte: SPI_XFER_1_DUAL
> switch to QUAD transfer after 4 bytes (e.g. 3byte address): SPI_XFER_4_QUAD
>
> Then only the flash driver needs to know these requirements, depending on
> manufacturer and chip type, and the spi driver just "reacts" on the flags.
> Otherwise this driver has to be extended each time a new command is added to
> the flash driver (as you did in some later patches!)
>

Thanks for your comments.
Yes, I agreed controller doesn't require to know about the flash instructions.
I am planning to remove these setup on next coming patch versions.

--
Thanks,
Jagan.