[PATCH 03/11] nand: spi: Abstract SPI NAND cmd set to functions

[Peter Pan]

Hi Arnaud,

On Tue, Feb 21, 2017 at 5:01 PM, Arnaud Mouiche
<arnaud.mouiche at gmail.com> wrote:
> Hello Peter.
> First thank you for this effort to finally bring the spinand framework back
> on stage.
>
> On 21/02/2017 09:00, Peter Pan wrote:
>>
>> This commit abstracts basic SPI NAND commands to
>> functions in spi-nand-base.c. Because command sets
>> have difference by vendors, we create spi-nand-cmd.c
>> to define this difference by command config table.
>>
>> [...]
>> +
>> +/**
>> + * spi_nand_read_page_to_cache - send command 13h to read data from Nand
>> + * to cache
>> + * @chip: SPI-NAND device structure
>> + * @page_addr: page to read
>> + */
>> +static int spi_nand_read_page_to_cache(struct spi_nand_chip *chip,
>> +                                       u32 page_addr)
>> +{
>> +       struct spi_nand_cmd cmd;
>> +
>> +       memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +       cmd.cmd = SPINAND_CMD_PAGE_READ;
>> +       cmd.n_addr = 3;
>> +       cmd.addr[0] = (u8)(page_addr >> 16);
>> +       cmd.addr[1] = (u8)(page_addr >> 8);
>> +       cmd.addr[2] = (u8)page_addr;
>> +
>> +       return spi_nand_issue_cmd(chip, &cmd);
>> +}
>
>
> Just a question. Don't you try to map too exactly the spi_nand_cmd to the
> internal micron SPI command ?
> Why "cmd.addr" should be a byte array, and not a u32, leaving each chip to
> details to handle the u32 to SPI byte stream mapping.
>
>
>
>> +
>> +/**
>> + * spi_nand_read_from_cache - read data out from cache register
>> + * @chip: SPI-NAND device structure
>> + * @page_addr: page to read
>> + * @column: the location to read from the cache
>> + * @len: number of bytes to read
>> + * @rbuf: buffer held @len bytes
>> + * Description:
>> + *   Command can be 03h, 0Bh, 3Bh, 6Bh, BBh, EBh
>> + *   The read can specify 1 to (page size + spare size) bytes of data
>> read at
>> + *   the corresponding locations.
>> + *   No tRd delay.
>> + */
>> +static int spi_nand_read_from_cache(struct spi_nand_chip *chip,
>> +               u32 page_addr, u32 column, size_t len, u8 *rbuf)
>> +{
>> +       struct nand_device *nand = &chip->base;
>> +       struct spi_nand_cmd cmd;
>> +
>> +       memset(&cmd, 0, sizeof(struct spi_nand_cmd));
>> +       cmd.cmd = chip->read_cache_op;
>> +       cmd.n_addr = 2;
>> +       cmd.addr[0] = (u8)(column >> 8);
>> +       if (chip->options & SPINAND_NEED_PLANE_SELECT)
>> +               cmd.addr[0] |= (u8)((nand_page_to_eraseblock(nand,
>> page_addr)
>> +                               & 0x1) << 4);
>> +       cmd.addr[1] = (u8)column;
>> +       cmd.n_rx = len;
>> +       cmd.rx_buf = rbuf;
>> +
>> +       return spi_nand_issue_cmd(chip, &cmd);
>> +}
>> +
>
> Some reasons to handle the plane concept (Micron specific), and even move
> the fact that it touch the bit 4 of cdm.addr[0], in common code ?

Let me try to remove this from framework when separate controller and device
related code. Any suggestion on this?

Thanks,
Peter Pan