Hi,
On Sat, May 18, 2019 at 6:05 PM Simon Glass sjg@chromium.org wrote:
In general we should avoid calling malloc() and free() repeatedly in U-Boot lest we turn it into tianocore. In SPL this can make SPI flash unusable since free() is often a nop and allocation space is limited.
In any case, these seems no need for malloc() since the number of bytes is very small, perhaps less than 8.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: d13f5b254a (spi: Extend the core to ease integration of SPI memory controllers)
drivers/spi/spi-mem.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index b86eee75bcb..7aabebeff5f 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -201,7 +201,6 @@ int spi_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;@@ -338,7 +337,17 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op) }
op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
op_buf = calloc(1, op_len);
/** Avoid using malloc() here so that we can use this code in SPL where* simple malloc may be used. That implementation does not allow free()* so repeated calls to this code can exhaust the space.** The value of op_len is small, since it does not include the actual* data being sent, only the op-code and address. In fact, it should be* possible to just use a small fixed value here instead of op_len.*/u8 op_buf[op_len];
I'd say just make this a fixed buffer instead of a VLA - less code space bloat and potential stack problems in case of nonsensical inputs.
As for the size, 8 bytes would be fine and actually leave some margin: the most i would expect for op_len is 1 + 4 + 1 = 6 bytes (the lowest would be 1+3+0 and the usual 1+3+1).