The size of the dynamic stack allocation here is bounded by the if() statement. However, just allocating the maximum size up-front and doing malloc() if necessary avoids code duplication (the i2c_setup_offset() until the invocation of ->xfer), and generates much better (smaller) code:
bloat-o-meter drivers/i2c/i2c-uclass.o.{0,1} add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-144 (-144) Function old new delta dm_i2c_write 552 408 -144 Total: Before=3828, After=3684, chg -3.76%
It also makes static analysis of maximum stack usage (using the .su files that are automatically generated during build) easier if there are no lines saying "dynamic".
[This is not entirely equivalent to the existing code; this now uses the stack for len <= 64 rather than len <= 63, but that seems like a more natural limit.]
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk --- drivers/i2c/i2c-uclass.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c index 71bc2b5b8a..a06553324e 100644 --- a/drivers/i2c/i2c-uclass.c +++ b/drivers/i2c/i2c-uclass.c @@ -168,6 +168,9 @@ int dm_i2c_write(struct udevice *dev, uint offset, const uint8_t *buffer, struct udevice *bus = dev_get_parent(dev); struct dm_i2c_ops *ops = i2c_get_ops(bus); struct i2c_msg msg[1]; + uint8_t _buf[I2C_MAX_OFFSET_LEN + 64]; + uint8_t *buf = _buf; + int ret;
if (!ops->xfer) return -ENOSYS; @@ -192,29 +195,20 @@ int dm_i2c_write(struct udevice *dev, uint offset, const uint8_t *buffer, * need to allow space for the offset (up to 4 bytes) and the message * itself. */ - if (len < 64) { - uint8_t buf[I2C_MAX_OFFSET_LEN + len]; - - i2c_setup_offset(chip, offset, buf, msg); - msg->len += len; - memcpy(buf + chip->offset_len, buffer, len); - - return ops->xfer(bus, msg, 1); - } else { - uint8_t *buf; - int ret; - + if (len > sizeof(_buf) - I2C_MAX_OFFSET_LEN) { buf = malloc(I2C_MAX_OFFSET_LEN + len); if (!buf) return -ENOMEM; - i2c_setup_offset(chip, offset, buf, msg); - msg->len += len; - memcpy(buf + chip->offset_len, buffer, len); + }
- ret = ops->xfer(bus, msg, 1); + i2c_setup_offset(chip, offset, buf, msg); + msg->len += len; + memcpy(buf + chip->offset_len, buffer, len); + + ret = ops->xfer(bus, msg, 1); + if (buf != _buf) free(buf); - return ret; - } + return ret; }
int dm_i2c_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs)