i2c: core: Remove needless structure member zero initialization

PATCH In review Jarkko Nikula 3 participants 8 messages Apr 29, 2015 – Jun 2, 2015 6 messages not in the archive

Subsystems (2)

I2C subsystem host drivers I2C

lore

Share & tools

lore

Jarkko Nikula

author

2015-04-29 12:44 UTC #0 lore raw

PATCH 1/2 i2c: core: Remove needless structure member zero initialization

No need to clear one struct i2c_client member variable since memset has already cleared all of them. Remove also one space intendation from err label.

Signed-off-by Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/[email protected]>

---
 drivers/i2c/i2c-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/i2c/i2c-core.c +1 -2

index 987c124432c5..051aa3a811a8 100644
@@ -283,7 +283,6 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 	memset(&client, 0, sizeof(client));
 	client.adapter = adapter;
 	client.addr = sb->slave_address;
-	client.flags = 0;
 
 	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
 		client.flags |= I2C_CLIENT_TEN;

@@ -361,7 +360,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 
 	gsb->status = status;
 
- err:
+err:
 	ACPI_FREE(ares);
 	return ret;
 }

Signature · 1 line

2.1.4

A message that is not in the archive (sent off-list or not yet received): 1430311477-21759-1-git-send-email-jarkko.nikula-VuQAYsv1563Yd54FQh9/[email protected]

Jarkko Nikula

author

2015-04-29 12:44 UTC #2 lore raw

PATCH 2/2 i2c: core: Reduce stack size of acpi_i2c_space_handler()

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-VuQAYsv1563Yd54FQh9/[email protected]>

---
 drivers/i2c/i2c-core.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

drivers/i2c/i2c-core.c +21 ~~-15~~

index 051aa3a811a8..3f25d758d83e 100644
@@ -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;
 }

Signature · 1 line

2.1.4

A message that is not in the archive (sent off-list or not yet received): 1430311477-21759-2-git-send-email-jarkko.nikula-VuQAYsv1563Yd54FQh9/[email protected]

Wolfram Sang

2015-05-12 19:05 UTC #4 lore raw

Re: [PATCH 2/2] i2c: core: Reduce stack size of acpi_i2c_space_handler()

Quoted 11 lines · Jarkko Nikula

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-VuQAYsv1563Yd54FQh9/[email protected]>

Besides it should be squashed with the previous patch, I'm fine with this. Mika?

Quoted 129 lines

---
 drivers/i2c/i2c-core.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

drivers/i2c/i2c-core.c +21 ~~-15~~

index 051aa3a811a8..3f25d758d83e 100644
@@ -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;
 }

Signature · 1 line

2.1.4

A message that is not in the archive (sent off-list or not yet received): 20150512190505.GB4449-oo5tB6JMkjKRinMKxDlMNPwbnWRJjS81@public.gmane.org

Mika Westerberg

Acked-by

2015-05-13 06:58 UTC #6 lore raw

Re: [PATCH 2/2] i2c: core: Reduce stack size of acpi_i2c_space_handler()

Quoted 8 lines · Wolfram Sang

Quoted 7 lines · Jarkko Nikula
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=]

Quoted 7 lines

Quoted 4 lines
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-VuQAYsv1563Yd54FQh9/[email protected]>
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-VuQAYsv1563Yd54FQh9/[email protected]>

A message that is not in the archive (sent off-list or not yet received): 20150513065812.GB1490-3PARRvDOhMZrdx17CPfAsdBPR1lH4CV8@public.gmane.org

Jarkko Nikula

author

2015-05-20 13:36 UTC #8 lore raw

PATCH v2 1/1 i2c: core: Reduce stack size of acpi_i2c_space_handler()

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-VuQAYsv1563Yd54FQh9/[email protected]>

Acked-by Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/[email protected]>

---
v2:
Ack from Mika added.
"- client.flags = 0;" removal squashed here and err label not touched.

---
 drivers/i2c/i2c-core.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

drivers/i2c/i2c-core.c +21 ~~-16~~

index 987c124432c5..ea60c1c89eb4 100644
@@ -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,75 +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.flags = 0;
+	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;

@@ -362,6 +366,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 	gsb->status = status;
 
  err:
+	kfree(client);
 	ACPI_FREE(ares);
 	return ret;
 }

Signature · 1 line

2.1.4

A message that is not in the archive (sent off-list or not yet received): 1432129012-26648-1-git-send-email-jarkko.nikula-VuQAYsv1563Yd54FQh9/[email protected]

Wolfram Sang

Applied

2015-06-02 15:29 UTC #10 lore raw

Re: [PATCH v2] i2c: core: Reduce stack size of acpi_i2c_space_handler()

Quoted 7 lines · Jarkko Nikula

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

Quoted 5 lines

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-VuQAYsv1563Yd54FQh9/[email protected]>
Acked-by Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/[email protected]>

Applied to for-next, thanks!

Wolfram Sang

2015-05-12 19:03 UTC #11 lore raw

In reply to a message not in the archive

Quoted 3 lines · Jarkko Nikula

No need to clear one struct i2c_client member variable since memset has already cleared all of them. Remove also one space intendation from err label.

Ehrm, why change this now when you remove the code block in the next patch anyhow? :)

Also, the indentation thing is very likely intentional. Sadly, I can't remember why at this moment, but I remember that when I tried to grep something I understood why some people put a space in front of labels

Quoted 34 lines

Signed-off-by Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/[email protected]>

---
 drivers/i2c/i2c-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/i2c/i2c-core.c +1 -2

index 987c124432c5..051aa3a811a8 100644
@@ -283,7 +283,6 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 	memset(&client, 0, sizeof(client));
 	client.adapter = adapter;
 	client.addr = sb->slave_address;
-	client.flags = 0;
 
 	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
 		client.flags |= I2C_CLIENT_TEN;

@@ -361,7 +360,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 
 	gsb->status = status;
 
- err:
+err:
 	ACPI_FREE(ares);
 	return ret;
 }

Signature · 6 lines

2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20150512190348.GA4449-oo5tB6JMkjKRinMKxDlMNPwbnWRJjS81@public.gmane.org

Jarkko Nikula

author

2015-05-13 06:10 UTC #13 lore raw

Quoted 8 lines · Wolfram Sang

Quoted 3 lines · Jarkko Nikula
No need to clear one struct i2c_client member variable since memset has already cleared all of them. Remove also one space intendation from err label.
Ehrm, why change this now when you remove the code block in the next patch anyhow? :)

Actually second patch is not removing any other code blocks than memset so my 2 seconds thinking said lets put that client.flags = 0; removal to another patch especially after finding the err label indentation, i.e. spring cleanings first and then the actual patch :-)

Quoted 4 lines

Also, the indentation thing is very likely intentional. Sadly, I can't remember why at this moment, but I remember that when I tried to grep something I understood why some people put a space in front of labels

Probably it makes code look better in some cases when there are multiple labels but here it didn't look consistent with the rest of the file.

I don't mind squashing this into second patch or keep them separate. I'd like to resend anyway because I noticed s/intendation/indentation/ misspelling in the commit log here.

Signature · 1 line

Jarkko