| Message ID | 1430311477-21759-2-git-send-email-jarkko.nikula@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Apr 29, 2015 at 03:44:37PM +0300, Jarkko Nikula wrote: > sizeof(struct i2c_client) is 1088 bytes on a CONFIG_X86_64=y build and > produces following warning when CONFIG_FRAME_WARN is set to 1024: > > drivers/i2c/i2c-core.c: In function ‘acpi_i2c_space_handler’: > drivers/i2c/i2c-core.c:367:1: warning: the frame size of 1152 bytes is > larger than 1024 bytes [-Wframe-larger-than=] > > This is not critical given that kernel stack is 16 kB on x86_64 but lets > reduce the stack usage by allocating the struct i2c_client from the heap. > > Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> Besides it should be squashed with the previous patch, I'm fine with this. Mika? > --- > drivers/i2c/i2c-core.c | 36 +++++++++++++++++++++--------------- > 1 file changed, 21 insertions(+), 15 deletions(-) > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c > index 051aa3a811a8..3f25d758d83e 100644 > --- a/drivers/i2c/i2c-core.c > +++ b/drivers/i2c/i2c-core.c > @@ -258,7 +258,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command, > struct acpi_connection_info *info = &data->info; > struct acpi_resource_i2c_serialbus *sb; > struct i2c_adapter *adapter = data->adapter; > - struct i2c_client client; > + struct i2c_client *client; > struct acpi_resource *ares; > u32 accessor_type = function >> 16; > u8 action = function & ACPI_IO_MASK; > @@ -269,6 +269,12 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command, > if (ACPI_FAILURE(ret)) > return ret; > > + client = kzalloc(sizeof(*client), GFP_KERNEL); > + if (!client) { > + ret = AE_NO_MEMORY; > + goto err; > + } > + > if (!value64 || ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) { > ret = AE_BAD_PARAMETER; > goto err; > @@ -280,74 +286,73 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command, > goto err; > } > > - memset(&client, 0, sizeof(client)); > - client.adapter = adapter; > - client.addr = sb->slave_address; > + client->adapter = adapter; > + client->addr = sb->slave_address; > > if (sb->access_mode == ACPI_I2C_10BIT_MODE) > - client.flags |= I2C_CLIENT_TEN; > + client->flags |= I2C_CLIENT_TEN; > > switch (accessor_type) { > case ACPI_GSB_ACCESS_ATTRIB_SEND_RCV: > if (action == ACPI_READ) { > - status = i2c_smbus_read_byte(&client); > + status = i2c_smbus_read_byte(client); > if (status >= 0) { > gsb->bdata = status; > status = 0; > } > } else { > - status = i2c_smbus_write_byte(&client, gsb->bdata); > + status = i2c_smbus_write_byte(client, gsb->bdata); > } > break; > > case ACPI_GSB_ACCESS_ATTRIB_BYTE: > if (action == ACPI_READ) { > - status = i2c_smbus_read_byte_data(&client, command); > + status = i2c_smbus_read_byte_data(client, command); > if (status >= 0) { > gsb->bdata = status; > status = 0; > } > } else { > - status = i2c_smbus_write_byte_data(&client, command, > + status = i2c_smbus_write_byte_data(client, command, > gsb->bdata); > } > break; > > case ACPI_GSB_ACCESS_ATTRIB_WORD: > if (action == ACPI_READ) { > - status = i2c_smbus_read_word_data(&client, command); > + status = i2c_smbus_read_word_data(client, command); > if (status >= 0) { > gsb->wdata = status; > status = 0; > } > } else { > - status = i2c_smbus_write_word_data(&client, command, > + status = i2c_smbus_write_word_data(client, command, > gsb->wdata); > } > break; > > case ACPI_GSB_ACCESS_ATTRIB_BLOCK: > if (action == ACPI_READ) { > - status = i2c_smbus_read_block_data(&client, command, > + status = i2c_smbus_read_block_data(client, command, > gsb->data); > if (status >= 0) { > gsb->len = status; > status = 0; > } > } else { > - status = i2c_smbus_write_block_data(&client, command, > + status = i2c_smbus_write_block_data(client, command, > gsb->len, gsb->data); > } > break; > > case ACPI_GSB_ACCESS_ATTRIB_MULTIBYTE: > if (action == ACPI_READ) { > - status = acpi_gsb_i2c_read_bytes(&client, command, > + status = acpi_gsb_i2c_read_bytes(client, command, > gsb->data, info->access_length); > if (status > 0) > status = 0; > } else { > - status = acpi_gsb_i2c_write_bytes(&client, command, > + status = acpi_gsb_i2c_write_bytes(client, command, > gsb->data, info->access_length); > } > break; > @@ -361,6 +366,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command, > gsb->status = status; > > err: > + kfree(client); > ACPI_FREE(ares); > return ret; > } > -- > 2.1.4 >
On Tue, May 12, 2015 at 09:05:05PM +0200, Wolfram Sang wrote: > On Wed, Apr 29, 2015 at 03:44:37PM +0300, Jarkko Nikula wrote: > > sizeof(struct i2c_client) is 1088 bytes on a CONFIG_X86_64=y build and > > produces following warning when CONFIG_FRAME_WARN is set to 1024: > > > > drivers/i2c/i2c-core.c: In function ‘acpi_i2c_space_handler’: > > drivers/i2c/i2c-core.c:367:1: warning: the frame size of 1152 bytes is > > larger than 1024 bytes [-Wframe-larger-than=] > > > > This is not critical given that kernel stack is 16 kB on x86_64 but lets > > reduce the stack usage by allocating the struct i2c_client from the heap. > > > > Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> > > Besides it should be squashed with the previous patch, I'm fine with > this. Mika? No objections from me, Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 051aa3a811a8..3f25d758d83e 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -258,7 +258,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command, struct acpi_connection_info *info = &data->info; struct acpi_resource_i2c_serialbus *sb; struct i2c_adapter *adapter = data->adapter; - struct i2c_client client; + struct i2c_client *client; struct acpi_resource *ares; u32 accessor_type = function >> 16; u8 action = function & ACPI_IO_MASK; @@ -269,6 +269,12 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command, if (ACPI_FAILURE(ret)) return ret; + client = kzalloc(sizeof(*client), GFP_KERNEL); + if (!client) { + ret = AE_NO_MEMORY; + goto err; + } + if (!value64 || ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) { ret = AE_BAD_PARAMETER; goto err; @@ -280,74 +286,73 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command, goto err; } - memset(&client, 0, sizeof(client)); - client.adapter = adapter; - client.addr = sb->slave_address; + client->adapter = adapter; + client->addr = sb->slave_address; if (sb->access_mode == ACPI_I2C_10BIT_MODE) - client.flags |= I2C_CLIENT_TEN; + client->flags |= I2C_CLIENT_TEN; switch (accessor_type) { case ACPI_GSB_ACCESS_ATTRIB_SEND_RCV: if (action == ACPI_READ) { - status = i2c_smbus_read_byte(&client); + status = i2c_smbus_read_byte(client); if (status >= 0) { gsb->bdata = status; status = 0; } } else { - status = i2c_smbus_write_byte(&client, gsb->bdata); + status = i2c_smbus_write_byte(client, gsb->bdata); } break; case ACPI_GSB_ACCESS_ATTRIB_BYTE: if (action == ACPI_READ) { - status = i2c_smbus_read_byte_data(&client, command); + status = i2c_smbus_read_byte_data(client, command); if (status >= 0) { gsb->bdata = status; status = 0; } } else { - status = i2c_smbus_write_byte_data(&client, command, + status = i2c_smbus_write_byte_data(client, command, gsb->bdata); } break; case ACPI_GSB_ACCESS_ATTRIB_WORD: if (action == ACPI_READ) { - status = i2c_smbus_read_word_data(&client, command); + status = i2c_smbus_read_word_data(client, command); if (status >= 0) { gsb->wdata = status; status = 0; } } else { - status = i2c_smbus_write_word_data(&client, command, + status = i2c_smbus_write_word_data(client, command, gsb->wdata); } break; case ACPI_GSB_ACCESS_ATTRIB_BLOCK: if (action == ACPI_READ) { - status = i2c_smbus_read_block_data(&client, command, + status = i2c_smbus_read_block_data(client, command, gsb->data); if (status >= 0) { gsb->len = status; status = 0; } } else { - status = i2c_smbus_write_block_data(&client, command, + status = i2c_smbus_write_block_data(client, command, gsb->len, gsb->data); } break; case ACPI_GSB_ACCESS_ATTRIB_MULTIBYTE: if (action == ACPI_READ) { - status = acpi_gsb_i2c_read_bytes(&client, command, + status = acpi_gsb_i2c_read_bytes(client, command, gsb->data, info->access_length); if (status > 0) status = 0; } else { - status = acpi_gsb_i2c_write_bytes(&client, command, + status = acpi_gsb_i2c_write_bytes(client, command, gsb->data, info->access_length); } break; @@ -361,6 +366,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command, gsb->status = status; err: + kfree(client); ACPI_FREE(ares); return ret; }
sizeof(struct i2c_client) is 1088 bytes on a CONFIG_X86_64=y build and produces following warning when CONFIG_FRAME_WARN is set to 1024: drivers/i2c/i2c-core.c: In function ‘acpi_i2c_space_handler’: drivers/i2c/i2c-core.c:367:1: warning: the frame size of 1152 bytes is larger than 1024 bytes [-Wframe-larger-than=] This is not critical given that kernel stack is 16 kB on x86_64 but lets reduce the stack usage by allocating the struct i2c_client from the heap. Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> --- drivers/i2c/i2c-core.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-)