i2c: avoid dynamic stack use in dm_i2c_write
authorRasmus Villemoes <rasmus.villemoes@prevas.dk>
Thu, 7 Jul 2022 13:12:17 +0000 (15:12 +0200)
committerHeiko Schocher <hs@denx.de>
Tue, 19 Jul 2022 11:46:28 +0000 (13:46 +0200)
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>
Reviewed-by: Heiko Schocher <hs@denx.de>
drivers/i2c/i2c-uclass.c

index 335911c..8d9a89e 100644 (file)
@@ -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)