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

[Arnaud Mouiche]

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 ?

Regards,
Arnaud