From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Date: Thu, 7 Jul 2022 13:12:17 +0000 (+0200)
Subject: i2c: avoid dynamic stack use in dm_i2c_write
X-Git-Url: http://git.dujemihanovic.xyz/html/%7B%7B%20%28.OutputFormats.Get?a=commitdiff_plain;h=49f3a42edf097fbaca76aef9bfcd98d871b442cb;p=u-boot.git

i2c: avoid dynamic stack use in dm_i2c_write

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>
---

diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index 335911c46b..8d9a89ed89 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)