| Message ID | f9aa39362e918b62aec0567f899b37d8d3c44710.1740064176.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | i2c: core: Allocate temporary client dynamically | expand |
On 2025/2/20 23:12, Geert Uytterhoeven wrote: > drivers/i2c/i2c-core-base.c: In function ‘i2c_detect.isra’: > drivers/i2c/i2c-core-base.c:2544:1: warning: the frame size of 1312 bytes is larger than 1024 bytes [-Wframe-larger-than=] > 2544 | } > | ^ > > Fix this by allocating the temporary client structure dynamically, as it > is a rather large structure (1216 bytes, depending on kernel config). > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Mostly compile-tested, as apparently I have no I2C devices for which > driver->detect and driver->address_list are valid. > > Apparently an alternative solution was posted before, but that does not > fully address the potential stack size issue: > "[PATCH] i2c: core: mark i2c_detect_address noinline_for_stack". > https://lore.kernel.org/20250210080217.2772467-1-suhui@nfschina.com > --- > drivers/i2c/i2c-core-base.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c > index 35a221e2c11c1460..7ad1ad5c8c3f5694 100644 > --- a/drivers/i2c/i2c-core-base.c > +++ b/drivers/i2c/i2c-core-base.c > @@ -2506,7 +2506,7 @@ static int i2c_detect_address(struct i2c_client *temp_client, > static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver) > { > const unsigned short *address_list; > - struct i2c_client temp_client; > + struct i2c_client *temp_client; > int i, err = 0; > > address_list = driver->address_list; > @@ -2527,19 +2527,24 @@ static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver) > return 0; > > /* Set up a temporary client to help detect callback */ > - memset(&temp_client, 0, sizeof(temp_client)); > - temp_client.adapter = adapter; > + temp_client = kzalloc(sizeof(*temp_client), GFP_KERNEL); > + if (!temp_client) > + return -ENOMEM; > + > + temp_client->adapter = adapter; > > for (i = 0; address_list[i] != I2C_CLIENT_END; i += 1) { > dev_dbg(&adapter->dev, > "found normal entry for adapter %d, addr 0x%02x\n", > i2c_adapter_id(adapter), address_list[i]); > - temp_client.addr = address_list[i]; > - err = i2c_detect_address(&temp_client, driver); > + temp_client->addr = address_list[i]; > + err = i2c_detect_address(temp_client, driver); > if (unlikely(err)) > break; > } > > + kfree(temp_client); > + > return err; > } Looks good to me. Reviewed-by: Su Hui <suhui@nfschina.com> Su Hui
On Thu, Feb 20, 2025 at 04:12:12PM +0100, Geert Uytterhoeven wrote: > drivers/i2c/i2c-core-base.c: In function ‘i2c_detect.isra’: > drivers/i2c/i2c-core-base.c:2544:1: warning: the frame size of 1312 bytes is larger than 1024 bytes [-Wframe-larger-than=] > 2544 | } > | ^ > > Fix this by allocating the temporary client structure dynamically, as it > is a rather large structure (1216 bytes, depending on kernel config). > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> As Arnd mentioned, this is basically a revert of 735668f8e5c9. Applied to for-current, thanks!
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 35a221e2c11c1460..7ad1ad5c8c3f5694 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -2506,7 +2506,7 @@ static int i2c_detect_address(struct i2c_client *temp_client, static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver) { const unsigned short *address_list; - struct i2c_client temp_client; + struct i2c_client *temp_client; int i, err = 0; address_list = driver->address_list; @@ -2527,19 +2527,24 @@ static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver) return 0; /* Set up a temporary client to help detect callback */ - memset(&temp_client, 0, sizeof(temp_client)); - temp_client.adapter = adapter; + temp_client = kzalloc(sizeof(*temp_client), GFP_KERNEL); + if (!temp_client) + return -ENOMEM; + + temp_client->adapter = adapter; for (i = 0; address_list[i] != I2C_CLIENT_END; i += 1) { dev_dbg(&adapter->dev, "found normal entry for adapter %d, addr 0x%02x\n", i2c_adapter_id(adapter), address_list[i]); - temp_client.addr = address_list[i]; - err = i2c_detect_address(&temp_client, driver); + temp_client->addr = address_list[i]; + err = i2c_detect_address(temp_client, driver); if (unlikely(err)) break; } + kfree(temp_client); + return err; }
drivers/i2c/i2c-core-base.c: In function ‘i2c_detect.isra’: drivers/i2c/i2c-core-base.c:2544:1: warning: the frame size of 1312 bytes is larger than 1024 bytes [-Wframe-larger-than=] 2544 | } | ^ Fix this by allocating the temporary client structure dynamically, as it is a rather large structure (1216 bytes, depending on kernel config). Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Mostly compile-tested, as apparently I have no I2C devices for which driver->detect and driver->address_list are valid. Apparently an alternative solution was posted before, but that does not fully address the potential stack size issue: "[PATCH] i2c: core: mark i2c_detect_address noinline_for_stack". https://lore.kernel.org/20250210080217.2772467-1-suhui@nfschina.com --- drivers/i2c/i2c-core-base.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)