From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Brezillon <boris.brezillon@bootlin.com>
Date: Thu, 29 Nov 2018 17:04:34 +0100
Subject: [U-Boot] [RFC PATCH 1/6] mtd: spi: Port SPI NOR framework from
	Linux
In-Reply-To: <20181128172607.10179-2-vigneshr@ti.com>
References: <20181128172607.10179-1-vigneshr@ti.com>
 <20181128172607.10179-2-vigneshr@ti.com>
Message-ID: <20181129170434.6db57b74@bbrezillon>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Wed, 28 Nov 2018 22:56:02 +0530
Vignesh R <vigneshr@ti.com> wrote:

> +#if defined(CONFIG_DM_SPI) && defined(CONFIG_SPI_MEM)
> +#define spi_nor_mem_exec_op spi_mem_exec_op
> +#else
> +/*
> + * This function is to support transition to DM_SPI. Will be removed
> + * once all boards are converted to DM_SPI
> + */
> +
> +static int spi_nor_mem_exec_op(struct spi_slave *slave,
> +			       const struct spi_mem_op *op)
> +{
> +	unsigned int pos = 0;
> +	const u8 *tx_buf = NULL;
> +	u8 *rx_buf = NULL;
> +	u8 *op_buf;
> +	int op_len;
> +	u32 flag;
> +	int ret;
> +	int i;
> +
> +	if (op->data.nbytes) {
> +		if (op->data.dir == SPI_MEM_DATA_IN)
> +			rx_buf = op->data.buf.in;
> +		else
> +			tx_buf = op->data.buf.out;
> +	}
> +
> +	op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
> +	op_buf = calloc(1, op_len);
> +
> +	ret = spi_claim_bus(slave);
> +	if (ret < 0)
> +		return ret;
> +
> +	op_buf[pos++] = op->cmd.opcode;
> +
> +	if (op->addr.nbytes) {
> +		for (i = 0; i < op->addr.nbytes; i++)
> +			op_buf[pos + i] = op->addr.val >>
> +				(8 * (op->addr.nbytes - i - 1));
> +
> +		pos += op->addr.nbytes;
> +	}
> +
> +	if (op->dummy.nbytes)
> +		memset(op_buf + pos, 0xff, op->dummy.nbytes);
> +
> +	/* 1st transfer: opcode + address + dummy cycles */
> +	flag = SPI_XFER_BEGIN;
> +	/* Make sure to set END bit if no tx or rx data messages follow */
> +	if (!tx_buf && !rx_buf)
> +		flag |= SPI_XFER_END;
> +
> +	ret = spi_xfer(slave, op_len * 8, op_buf, NULL, flag);
> +	if (ret)
> +		return ret;
> +
> +	/* 2nd transfer: rx or tx data path */
> +	if (tx_buf || rx_buf) {
> +		ret = spi_xfer(slave, op->data.nbytes * 8, tx_buf,
> +			       rx_buf, SPI_XFER_END);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	spi_release_bus(slave);
> +
> +	for (i = 0; i < pos; i++)
> +		debug("%02x ", op_buf[i]);
> +	debug("| [%dB %s] ",
> +	      tx_buf || rx_buf ? op->data.nbytes : 0,
> +	      tx_buf || rx_buf ? (tx_buf ? "out" : "in") : "-");
> +	for (i = 0; i < op->data.nbytes; i++)
> +		debug("%02x ", tx_buf ? tx_buf[i] : rx_buf[i]);
> +	debug("[ret %d]\n", ret);
> +
> +	free(op_buf);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +#endif

I'd recommend putting this alternate spi_mem_exec_op() implementation
in drivers/spi/. Either you create a drivers/spi/spi-mem-no-dm.c file
or you add #ifdef CONFIG_DM_SPI #else #endif sections directly in
drivers/spi/spi-mem.c.

> +const struct flash_info spi_nor_ids[] = {
> +#ifdef CONFIG_SPI_FLASH_ATMEL		/* ATMEL */
> +	/* Atmel -- some are (confusingly) marketed as "DataFlash" */
> +	{ "at26df321",  INFO(0x1f4700, 0, 64 * 1024, 64, SECT_4K) },
> +	{ "at25df321a", INFO(0x1f4701, 0, 64 * 1024, 64, SECT_4K) },
> +
> +	{ "at45db011d",	INFO(0x1f2200, 0, 64 * 1024,   4, SECT_4K) },
> +	{ "at45db021d",	INFO(0x1f2300, 0, 64 * 1024,   8, SECT_4K) },
> +	{ "at45db041d",	INFO(0x1f2400, 0, 64 * 1024,   8, SECT_4K) },
> +	{ "at45db081d", INFO(0x1f2500, 0, 64 * 1024,  16, SECT_4K) },
> +	{ "at45db161d",	INFO(0x1f2600, 0, 64 * 1024,  32, SECT_4K) },
> +	{ "at45db321d",	INFO(0x1f2700, 0, 64 * 1024,  64, SECT_4K) },
> +	{ "at45db641d",	INFO(0x1f2800, 0, 64 * 1024, 128, SECT_4K) },
> +	{ "at26df081a", INFO(0x1f4501, 0, 64 * 1024,  16, SECT_4K) },
> +#endif

If you're really short in space (for the SPL build), you might want to
consider fined-grained selection of the chips you want to support. One
more reason to do that is that board manufacturers usually source SPI
NOR parts from different vendors for the same design. With a
per-manufacturer selection logic, you'll have to enable several of them
to have an SPL that works on all variants.

I didn't try, but you might be able to place each NOR chip in its own
section and decide which one to keep at link time (will require some
macros to define flash_info entries + a linker script to decide which
sections you want to discard/keep at link time).

Note that I'd be okay to convert Linux flash_info table to the new
macro usage if you choose to do that.

Just a quick example:

#define SPI_FLASH_INFO(_name, ...)
	ll_entry_declare(struct flash_info, _name, spi_nor_ids) = {
		.name = #_name,
		__VAR_ARGS__
	};