From: Arnd Bergmann arnd@arndb.de
Storing a bogus i2c_client structure on the stack adds overhead and causes a compile-time warning:
drivers/tee/optee/rpc.c:493:6: error: stack frame size of 1056 bytes in function 'optee_handle_rpc' [-Werror,-Wframe-larger-than=] void optee_handle_rpc(struct tee_context *ctx, struct optee_rpc_param *param,
Change the implementation of handle_rpc_func_cmd_i2c_transfer() to open-code the i2c_transfer() call, which makes it easier to read and avoids the warning.
Fixes: c05210ab9757 ("drivers: optee: allow op-tee to access devices on the i2c bus") Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/tee/optee/rpc.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c index 1e3614e4798f..6cbb3643c6c4 100644 --- a/drivers/tee/optee/rpc.c +++ b/drivers/tee/optee/rpc.c @@ -54,8 +54,9 @@ static void handle_rpc_func_cmd_get_time(struct optee_msg_arg *arg) static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, struct optee_msg_arg *arg) { - struct i2c_client client = { 0 }; struct tee_param *params; + struct i2c_adapter *adapter; + struct i2c_msg msg = { }; size_t i; int ret = -EOPNOTSUPP; u8 attr[] = { @@ -85,48 +86,48 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, goto bad; }
- client.adapter = i2c_get_adapter(params[0].u.value.b); - if (!client.adapter) + adapter = i2c_get_adapter(params[0].u.value.b); + if (!adapter) goto bad;
if (params[1].u.value.a & OPTEE_MSG_RPC_CMD_I2C_FLAGS_TEN_BIT) { - if (!i2c_check_functionality(client.adapter, + if (!i2c_check_functionality(adapter, I2C_FUNC_10BIT_ADDR)) { - i2c_put_adapter(client.adapter); + i2c_put_adapter(adapter); goto bad; }
- client.flags = I2C_CLIENT_TEN; + msg.flags = I2C_M_TEN; }
- client.addr = params[0].u.value.c; - snprintf(client.name, I2C_NAME_SIZE, "i2c%d", client.adapter->nr); + msg.addr = params[0].u.value.c; + msg.buf = params[2].u.memref.shm->kaddr; + msg.len = params[2].u.memref.size;
switch (params[0].u.value.a) { case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD: - ret = i2c_master_recv(&client, params[2].u.memref.shm->kaddr, - params[2].u.memref.size); + msg.flags |= I2C_M_RD; break; case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR: - ret = i2c_master_send(&client, params[2].u.memref.shm->kaddr, - params[2].u.memref.size); break; default: - i2c_put_adapter(client.adapter); + i2c_put_adapter(adapter); goto bad; }
+ ret = i2c_transfer(adapter, &msg, 1); + if (ret < 0) { arg->ret = TEEC_ERROR_COMMUNICATION; } else { - params[3].u.value.a = ret; + params[3].u.value.a = msg.len; if (optee_to_msg_param(arg->params, arg->num_params, params)) arg->ret = TEEC_ERROR_BAD_PARAMETERS; else arg->ret = TEEC_SUCCESS; }
- i2c_put_adapter(client.adapter); + i2c_put_adapter(adapter); kfree(params); return; bad:
On 25/01/21, Arnd Bergmann wrote:
From: Arnd Bergmann arnd@arndb.de
Storing a bogus i2c_client structure on the stack adds overhead and causes a compile-time warning:
drivers/tee/optee/rpc.c:493:6: error: stack frame size of 1056 bytes in function 'optee_handle_rpc' [-Werror,-Wframe-larger-than=] void optee_handle_rpc(struct tee_context *ctx, struct optee_rpc_param *param,
Change the implementation of handle_rpc_func_cmd_i2c_transfer() to open-code the i2c_transfer() call, which makes it easier to read and avoids the warning.
Fixes: c05210ab9757 ("drivers: optee: allow op-tee to access devices on the i2c bus")
does fixing stack-frame compile warnings need a 'fixes' tag?
Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/tee/optee/rpc.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c index 1e3614e4798f..6cbb3643c6c4 100644 --- a/drivers/tee/optee/rpc.c +++ b/drivers/tee/optee/rpc.c @@ -54,8 +54,9 @@ static void handle_rpc_func_cmd_get_time(struct optee_msg_arg *arg) static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, struct optee_msg_arg *arg) {
- struct i2c_client client = { 0 }; struct tee_param *params;
- struct i2c_adapter *adapter;
- struct i2c_msg msg = { }; size_t i; int ret = -EOPNOTSUPP; u8 attr[] = {
@@ -85,48 +86,48 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, goto bad; }
- client.adapter = i2c_get_adapter(params[0].u.value.b);
- if (!client.adapter)
- adapter = i2c_get_adapter(params[0].u.value.b);
- if (!adapter) goto bad;
if (params[1].u.value.a & OPTEE_MSG_RPC_CMD_I2C_FLAGS_TEN_BIT) {
if (!i2c_check_functionality(client.adapter,
if (!i2c_check_functionality(adapter, I2C_FUNC_10BIT_ADDR)) {
i2c_put_adapter(client.adapter);
i2c_put_adapter(adapter); goto bad;
client.flags = I2C_CLIENT_TEN;
msg.flags = I2C_M_TEN;
- client.addr = params[0].u.value.c;
- snprintf(client.name, I2C_NAME_SIZE, "i2c%d", client.adapter->nr);
- msg.addr = params[0].u.value.c;
- msg.buf = params[2].u.memref.shm->kaddr;
- msg.len = params[2].u.memref.size;
switch (params[0].u.value.a) { case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD:
ret = i2c_master_recv(&client, params[2].u.memref.shm->kaddr,params[2].u.memref.size);
msg.flags |= I2C_M_RD;
ret = i2c_master_send(&client, params[2].u.memref.shm->kaddr,params[2].u.memref.size);i2c_put_adapter(client.adapter);
i2c_put_adapter(adapter);
- ret = i2c_transfer(adapter, &msg, 1);
- if (ret < 0) { arg->ret = TEEC_ERROR_COMMUNICATION; } else {
params[3].u.value.a = ret;
params[3].u.value.a = msg.len;
- i2c_put_adapter(client.adapter);
- i2c_put_adapter(adapter); kfree(params); return;
bad:
2.29.2
On Tue, Jan 26, 2021 at 9:08 AM Jorge Ramirez-Ortiz, Foundries jorge@foundries.io wrote:
On 25/01/21, Arnd Bergmann wrote:
From: Arnd Bergmann arnd@arndb.de
Storing a bogus i2c_client structure on the stack adds overhead and causes a compile-time warning:
drivers/tee/optee/rpc.c:493:6: error: stack frame size of 1056 bytes in function 'optee_handle_rpc' [-Werror,-Wframe-larger-than=] void optee_handle_rpc(struct tee_context *ctx, struct optee_rpc_param *param,
Change the implementation of handle_rpc_func_cmd_i2c_transfer() to open-code the i2c_transfer() call, which makes it easier to read and avoids the warning.
Fixes: c05210ab9757 ("drivers: optee: allow op-tee to access devices on the i2c bus")
does fixing stack-frame compile warnings need a 'fixes' tag?
The fixes tag only describes which commit introduced the bug, it is irrelevant what type of bug this is.
Arnd
On 26/01/21, Arnd Bergmann wrote:
On Tue, Jan 26, 2021 at 9:08 AM Jorge Ramirez-Ortiz, Foundries jorge@foundries.io wrote:
On 25/01/21, Arnd Bergmann wrote:
From: Arnd Bergmann arnd@arndb.de
Storing a bogus i2c_client structure on the stack adds overhead and causes a compile-time warning:
drivers/tee/optee/rpc.c:493:6: error: stack frame size of 1056 bytes in function 'optee_handle_rpc' [-Werror,-Wframe-larger-than=] void optee_handle_rpc(struct tee_context *ctx, struct optee_rpc_param *param,
Change the implementation of handle_rpc_func_cmd_i2c_transfer() to open-code the i2c_transfer() call, which makes it easier to read and avoids the warning.
Fixes: c05210ab9757 ("drivers: optee: allow op-tee to access devices on the i2c bus")
does fixing stack-frame compile warnings need a 'fixes' tag?
The fixes tag only describes which commit introduced the bug, it is irrelevant what type of bug this is.
Arnd
thanks Arnd.
what compiler warnings are defined as kernel bugs? is there a list that you use when tracking these?
On Tue, Jan 26, 2021 at 12:50 PM Jorge Ramirez-Ortiz, Foundries jorge@foundries.io wrote:
On 26/01/21, Arnd Bergmann wrote:
On Tue, Jan 26, 2021 at 9:08 AM Jorge Ramirez-Ortiz, Foundries
The fixes tag only describes which commit introduced the bug, it is irrelevant what type of bug this is.
thanks Arnd.
what compiler warnings are defined as kernel bugs? is there a list that you use when tracking these?
I consider any warning a bug, a normal kernel build should always be warning free.
I sometimes send fixes for warnings that only happen with 'make W=1', 'make C=1' or even 'make W=2'. For those, I would only categorize them as a real bug if they actually cause runtime misbehavior, but there are some -Wsomething flags that we would like to enable by default in the future.
Arnd
Hi Arnd,
On Mon, Jan 25, 2021 at 12:38 PM Arnd Bergmann arnd@kernel.org wrote:
From: Arnd Bergmann arnd@arndb.de
Storing a bogus i2c_client structure on the stack adds overhead and causes a compile-time warning:
drivers/tee/optee/rpc.c:493:6: error: stack frame size of 1056 bytes in function 'optee_handle_rpc' [-Werror,-Wframe-larger-than=] void optee_handle_rpc(struct tee_context *ctx, struct optee_rpc_param *param,
Change the implementation of handle_rpc_func_cmd_i2c_transfer() to open-code the i2c_transfer() call, which makes it easier to read and avoids the warning.
Fixes: c05210ab9757 ("drivers: optee: allow op-tee to access devices on the i2c bus") Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/tee/optee/rpc.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
Looks good to me. Reviewed-by: Jens Wiklander jens.wiklander@linaro.org
Do you want to take it up directly yourself or do you want a pull request from me?
Thanks, Jens
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c index 1e3614e4798f..6cbb3643c6c4 100644 --- a/drivers/tee/optee/rpc.c +++ b/drivers/tee/optee/rpc.c @@ -54,8 +54,9 @@ static void handle_rpc_func_cmd_get_time(struct optee_msg_arg *arg) static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, struct optee_msg_arg *arg) {
struct i2c_client client = { 0 }; struct tee_param *params;
struct i2c_adapter *adapter;struct i2c_msg msg = { }; size_t i; int ret = -EOPNOTSUPP; u8 attr[] = {@@ -85,48 +86,48 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx, goto bad; }
client.adapter = i2c_get_adapter(params[0].u.value.b);if (!client.adapter)
adapter = i2c_get_adapter(params[0].u.value.b);if (!adapter) goto bad; if (params[1].u.value.a & OPTEE_MSG_RPC_CMD_I2C_FLAGS_TEN_BIT) {
if (!i2c_check_functionality(client.adapter,
if (!i2c_check_functionality(adapter, I2C_FUNC_10BIT_ADDR)) {
i2c_put_adapter(client.adapter);
i2c_put_adapter(adapter); goto bad; }
client.flags = I2C_CLIENT_TEN;
msg.flags = I2C_M_TEN; }
client.addr = params[0].u.value.c;snprintf(client.name, I2C_NAME_SIZE, "i2c%d", client.adapter->nr);
msg.addr = params[0].u.value.c;msg.buf = params[2].u.memref.shm->kaddr;msg.len = params[2].u.memref.size; switch (params[0].u.value.a) { case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD:
ret = i2c_master_recv(&client, params[2].u.memref.shm->kaddr,params[2].u.memref.size);
msg.flags |= I2C_M_RD; break; case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR:
ret = i2c_master_send(&client, params[2].u.memref.shm->kaddr,params[2].u.memref.size); break; default:i2c_put_adapter(client.adapter);
i2c_put_adapter(adapter); goto bad; }ret = i2c_transfer(adapter, &msg, 1);if (ret < 0) { arg->ret = TEEC_ERROR_COMMUNICATION; } else {
params[3].u.value.a = ret;
params[3].u.value.a = msg.len; if (optee_to_msg_param(arg->params, arg->num_params, params)) arg->ret = TEEC_ERROR_BAD_PARAMETERS; else arg->ret = TEEC_SUCCESS; }
i2c_put_adapter(client.adapter);
i2c_put_adapter(adapter); kfree(params); return;bad:
2.29.2
Hi Jorge,
On Wed, Jan 27, 2021 at 11:41 AM Jens Wiklander jens.wiklander@linaro.org wrote:
Hi Arnd,
On Mon, Jan 25, 2021 at 12:38 PM Arnd Bergmann arnd@kernel.org wrote:
From: Arnd Bergmann arnd@arndb.de
Storing a bogus i2c_client structure on the stack adds overhead and causes a compile-time warning:
drivers/tee/optee/rpc.c:493:6: error: stack frame size of 1056 bytes in function 'optee_handle_rpc' [-Werror,-Wframe-larger-than=] void optee_handle_rpc(struct tee_context *ctx, struct optee_rpc_param *param,
Change the implementation of handle_rpc_func_cmd_i2c_transfer() to open-code the i2c_transfer() call, which makes it easier to read and avoids the warning.
Fixes: c05210ab9757 ("drivers: optee: allow op-tee to access devices on the i2c bus") Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/tee/optee/rpc.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
Looks good to me. Reviewed-by: Jens Wiklander jens.wiklander@linaro.org
Would you mind testing this?
Thanks, Jens
On 08/02/21, Jens Wiklander wrote:
Hi Jorge,
On Wed, Jan 27, 2021 at 11:41 AM Jens Wiklander jens.wiklander@linaro.org wrote:
Hi Arnd,
On Mon, Jan 25, 2021 at 12:38 PM Arnd Bergmann arnd@kernel.org wrote:
From: Arnd Bergmann arnd@arndb.de
Storing a bogus i2c_client structure on the stack adds overhead and causes a compile-time warning:
drivers/tee/optee/rpc.c:493:6: error: stack frame size of 1056 bytes in function 'optee_handle_rpc' [-Werror,-Wframe-larger-than=] void optee_handle_rpc(struct tee_context *ctx, struct optee_rpc_param *param,
Change the implementation of handle_rpc_func_cmd_i2c_transfer() to open-code the i2c_transfer() call, which makes it easier to read and avoids the warning.
Fixes: c05210ab9757 ("drivers: optee: allow op-tee to access devices on the i2c bus") Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/tee/optee/rpc.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
Looks good to me. Reviewed-by: Jens Wiklander jens.wiklander@linaro.org
Would you mind testing this?
sure, doing it this morning.
btw what Arnd has done - removing the unnecessary level of indirection - was pretty much my initial though but I thought it was easier to read the way I wrote it (I guess I was wrong and I obviously missed the stack size increase)
but yes, will test
Thanks, Jens
On 08/02/21, Jorge Ramirez-Ortiz, Foundries wrote:
On 08/02/21, Jens Wiklander wrote:
Hi Jorge,
On Wed, Jan 27, 2021 at 11:41 AM Jens Wiklander jens.wiklander@linaro.org wrote:
Hi Arnd,
On Mon, Jan 25, 2021 at 12:38 PM Arnd Bergmann arnd@kernel.org wrote:
From: Arnd Bergmann arnd@arndb.de
Storing a bogus i2c_client structure on the stack adds overhead and causes a compile-time warning:
drivers/tee/optee/rpc.c:493:6: error: stack frame size of 1056 bytes in function 'optee_handle_rpc' [-Werror,-Wframe-larger-than=] void optee_handle_rpc(struct tee_context *ctx, struct optee_rpc_param *param,
Change the implementation of handle_rpc_func_cmd_i2c_transfer() to open-code the i2c_transfer() call, which makes it easier to read and avoids the warning.
Fixes: c05210ab9757 ("drivers: optee: allow op-tee to access devices on the i2c bus") Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/tee/optee/rpc.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
Looks good to me. Reviewed-by: Jens Wiklander jens.wiklander@linaro.org
Would you mind testing this?
sure, doing it this morning.
btw what Arnd has done - removing the unnecessary level of indirection
- was pretty much my initial though but I thought it was easier to
read the way I wrote it (I guess I was wrong and I obviously missed the stack size increase)
but yes, will test
Tested on imx6ull.
Tested-by: Jorge Ramirez-Ortiz jorge@foundries.io
Thanks, Jens
On Mon, Feb 8, 2021 at 9:32 AM Jorge Ramirez-Ortiz, Foundries jorge@foundries.io wrote:
On 08/02/21, Jorge Ramirez-Ortiz, Foundries wrote:
On 08/02/21, Jens Wiklander wrote:
Hi Jorge,
On Wed, Jan 27, 2021 at 11:41 AM Jens Wiklander jens.wiklander@linaro.org wrote:
Hi Arnd,
On Mon, Jan 25, 2021 at 12:38 PM Arnd Bergmann arnd@kernel.org wrote:
From: Arnd Bergmann arnd@arndb.de
Storing a bogus i2c_client structure on the stack adds overhead and causes a compile-time warning:
drivers/tee/optee/rpc.c:493:6: error: stack frame size of 1056 bytes in function 'optee_handle_rpc' [-Werror,-Wframe-larger-than=] void optee_handle_rpc(struct tee_context *ctx, struct optee_rpc_param *param,
Change the implementation of handle_rpc_func_cmd_i2c_transfer() to open-code the i2c_transfer() call, which makes it easier to read and avoids the warning.
Fixes: c05210ab9757 ("drivers: optee: allow op-tee to access devices on the i2c bus") Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/tee/optee/rpc.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
Looks good to me. Reviewed-by: Jens Wiklander jens.wiklander@linaro.org
Would you mind testing this?
sure, doing it this morning.
btw what Arnd has done - removing the unnecessary level of indirection
- was pretty much my initial though but I thought it was easier to
read the way I wrote it (I guess I was wrong and I obviously missed the stack size increase)
but yes, will test
Tested on imx6ull.
Tested-by: Jorge Ramirez-Ortiz jorge@foundries.io
Thank you.
Cheers, Jens
op-tee@lists.trustedfirmware.org
-
Arnd Bergmann -
Jens Wiklander -
Jorge Ramirez-Ortiz, Foundries