Hi Boris,
On 29/11/18 9:34 PM, Boris Brezillon wrote:
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;elsetx_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.
I thought about that, but then I saw spi-mem.c is under DM_SPI and DM_SPI migration series by Jagan and thought that this code wont be needing this at all. Anyways, will move this to drivers/spi next time around.
+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).
IIUC, you are proposing per board linker script that will select/choose which flash parts would to be supported by that board's SPL?
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__ };