Re: [PATCH 10/11] nand: spi: Add generic SPI controller support

Peter Pan
2017-02-21T09:37:54Z

Hi Arnaud,

On Tue, Feb 21, 2017 at 5:25 PM, Arnaud Mouiche
<arnaud.mouiche@gmail.com> wrote:
>
>
> On 21/02/2017 09:00, Peter Pan wrote:
>>
>> This commit supports to use generic spi controller
>> as SPI NAND controller.
>>
>> Signed-off-by: Peter Pan <peterpandong@micron.com>
>> ---
>> drivers/mtd/nand/spi/Kconfig | 2 +
>> drivers/mtd/nand/spi/Makefile | 1 +
>> drivers/mtd/nand/spi/chips/Kconfig | 5 +
>> drivers/mtd/nand/spi/chips/Makefile | 1 +
>> drivers/mtd/nand/spi/chips/generic_spi.c | 269
>> +++++++++++++++++++++++++++++++
>> 5 files changed, 278 insertions(+)
>> create mode 100644 drivers/mtd/nand/spi/chips/Kconfig
>> create mode 100644 drivers/mtd/nand/spi/chips/Makefile
>> create mode 100644 drivers/mtd/nand/spi/chips/generic_spi.c
>
> [...]
>
>> +/*
>> + * generic_spi_nand_cmd_fn - to process a command to send to the SPI-NAND
>> + * by generic SPI bus
>> + * @chip: SPI-NAND device structure
>> + * @cmd: command structure
>> + * Description:
>> + * Set up the command buffer to send to the SPI controller.
>> + * The command buffer has to initialized to 0.
>> + */
>> +static int generic_spi_nand_cmd_fn(struct spi_nand_chip *chip,
>> + struct spi_nand_cmd *cmd)
>> +{
>> + struct spi_nand_cmd_cfg *cmd_cfg = spi_nand_get_cmd_cfg(cmd->cmd);
>> + struct spi_message message;
>> + struct spi_transfer x[3];
>> + struct spi_device *spi = chip->priv;
>> + u8 buf[SPINAND_MAX_ADDR_LEN];
>> +
>> + spi_message_init(&message);
>> + memset(x, 0, sizeof(x));
>> + x[0].len = 1;
>> + x[0].tx_nbits = 1;
>> + x[0].tx_buf = &cmd->cmd;
>> + spi_message_add_tail(&x[0], &message);
>> +
>> + if (cmd_cfg->addr_bytes || cmd_cfg->dummy_bytes) {
>> + if (cmd_cfg->addr_bytes > cmd->n_addr) {
>> + memcpy(buf, cmd->addr, cmd->n_addr);
>> + memset(cmd->addr, 0, cmd->n_addr);
>> + memcpy(cmd->addr + cmd_cfg->addr_bytes -
>> cmd->n_addr,
>> + buf, cmd->n_addr);
>> + }
>> + x[1].len = cmd_cfg->addr_bytes + cmd_cfg->dummy_bytes;
>> + x[1].tx_nbits = cmd_cfg->addr_bits;
>> + x[1].tx_buf = cmd->addr;
>> + spi_message_add_tail(&x[1], &message);
>> + }
>> + if (cmd->n_tx) {
>> + x[2].len = cmd->n_tx;
>> + x[2].tx_nbits = cmd_cfg->data_bits;
>> + x[2].tx_buf = cmd->tx_buf;
>> + spi_message_add_tail(&x[2], &message);
>> + } else if (cmd->n_rx) {
>> + x[2].len = cmd->n_rx;
>> + x[2].rx_nbits = cmd_cfg->data_bits;
>> + x[2].rx_buf = cmd->rx_buf;
>> + spi_message_add_tail(&x[2], &message);
>> + }
>> + return spi_sync(spi, &message);
>> +}
>> +
>>
> Just a spi speed consideration.
> All the spi drivers I've work with are not very good for scatter/gather, and
> performance are dropping especially when you gather only 1 to 3 bytes parts.
> But we can manage this optimization later anyway, with proper speed figures,
> once the spinand framework will be merged.

I also found the speed issue when debugging. For opcode and address transfer
and get/set feature, CPU polling is better than DMA while CPU resource
is wasted.

Thanks,
Peter Pan