Re: [PATCH] optee: simplify i2c access

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