All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] optee: simplify i2c access
@ 2021-01-25 11:37 Arnd Bergmann
  2021-01-26  8:08 ` Jorge Ramirez-Ortiz, Foundries
  2021-01-27 10:41 ` Jens Wiklander
  0 siblings, 2 replies; 10+ messages in thread
From: Arnd Bergmann @ 2021-01-25 11:37 UTC (permalink / raw)
  To: Jens Wiklander, Jorge Ramirez-Ortiz; +Cc: Arnd Bergmann, op-tee, linux-kernel

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:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] optee: simplify i2c access
  2021-01-25 11:37 [PATCH] optee: simplify i2c access Arnd Bergmann
@ 2021-01-26  8:08 ` Jorge Ramirez-Ortiz, Foundries
  2021-01-26  9:10   ` Arnd Bergmann
  2021-01-27 10:41 ` Jens Wiklander
  1 sibling, 1 reply; 10+ messages in thread
From: Jorge Ramirez-Ortiz, Foundries @ 2021-01-26  8:08 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jens Wiklander, Jorge Ramirez-Ortiz, Arnd Bergmann, op-tee, linux-kernel

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;
>  		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
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] optee: simplify i2c access
  2021-01-26  8:08 ` Jorge Ramirez-Ortiz, Foundries
@ 2021-01-26  9:10   ` Arnd Bergmann
  2021-01-26 11:45     ` Jorge Ramirez-Ortiz, Foundries
  0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2021-01-26  9:10 UTC (permalink / raw)
  To: Jorge Ramirez-Ortiz, Foundries
  Cc: Jens Wiklander, Arnd Bergmann, op-tee, linux-kernel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] optee: simplify i2c access
  2021-01-26  9:10   ` Arnd Bergmann
@ 2021-01-26 11:45     ` Jorge Ramirez-Ortiz, Foundries
  2021-01-26 14:09       ` Arnd Bergmann
  0 siblings, 1 reply; 10+ messages in thread
From: Jorge Ramirez-Ortiz, Foundries @ 2021-01-26 11:45 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jorge Ramirez-Ortiz, Foundries, Jens Wiklander, Arnd Bergmann,
	op-tee, linux-kernel

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?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] optee: simplify i2c access
  2021-01-26 11:45     ` Jorge Ramirez-Ortiz, Foundries
@ 2021-01-26 14:09       ` Arnd Bergmann
  0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2021-01-26 14:09 UTC (permalink / raw)
  To: Jorge Ramirez-Ortiz, Foundries
  Cc: Jens Wiklander, Arnd Bergmann, op-tee, linux-kernel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] optee: simplify i2c access
  2021-01-25 11:37 [PATCH] optee: simplify i2c access Arnd Bergmann
  2021-01-26  8:08 ` Jorge Ramirez-Ortiz, Foundries
@ 2021-01-27 10:41 ` Jens Wiklander
  2021-02-08  7:00   ` Jens Wiklander
  1 sibling, 1 reply; 10+ messages in thread
From: Jens Wiklander @ 2021-01-27 10:41 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jorge Ramirez-Ortiz, Arnd Bergmann, op-tee, Linux Kernel Mailing List

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
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] optee: simplify i2c access
  2021-01-27 10:41 ` Jens Wiklander
@ 2021-02-08  7:00   ` Jens Wiklander
  2021-02-08  7:46     ` Jorge Ramirez-Ortiz, Foundries
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Wiklander @ 2021-02-08  7:00 UTC (permalink / raw)
  To: Jorge Ramirez-Ortiz
  Cc: Arnd Bergmann, Arnd Bergmann, op-tee, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] optee: simplify i2c access
  2021-02-08  7:00   ` Jens Wiklander
@ 2021-02-08  7:46     ` Jorge Ramirez-Ortiz, Foundries
  2021-02-08  8:32       ` Jorge Ramirez-Ortiz, Foundries
  0 siblings, 1 reply; 10+ messages in thread
From: Jorge Ramirez-Ortiz, Foundries @ 2021-02-08  7:46 UTC (permalink / raw)
  To: Jens Wiklander
  Cc: Jorge Ramirez-Ortiz, Arnd Bergmann, Arnd Bergmann, op-tee,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] optee: simplify i2c access
  2021-02-08  7:46     ` Jorge Ramirez-Ortiz, Foundries
@ 2021-02-08  8:32       ` Jorge Ramirez-Ortiz, Foundries
  2021-02-08  8:54         ` Jens Wiklander
  0 siblings, 1 reply; 10+ messages in thread
From: Jorge Ramirez-Ortiz, Foundries @ 2021-02-08  8:32 UTC (permalink / raw)
  To: Jorge Ramirez-Ortiz, Foundries
  Cc: Jens Wiklander, Arnd Bergmann, Arnd Bergmann, op-tee,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] optee: simplify i2c access
  2021-02-08  8:32       ` Jorge Ramirez-Ortiz, Foundries
@ 2021-02-08  8:54         ` Jens Wiklander
  0 siblings, 0 replies; 10+ messages in thread
From: Jens Wiklander @ 2021-02-08  8:54 UTC (permalink / raw)
  To: Jorge Ramirez-Ortiz, Foundries
  Cc: Arnd Bergmann, Arnd Bergmann, op-tee, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-02-08  9:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-25 11:37 [PATCH] optee: simplify i2c access Arnd Bergmann
2021-01-26  8:08 ` Jorge Ramirez-Ortiz, Foundries
2021-01-26  9:10   ` Arnd Bergmann
2021-01-26 11:45     ` Jorge Ramirez-Ortiz, Foundries
2021-01-26 14:09       ` Arnd Bergmann
2021-01-27 10:41 ` Jens Wiklander
2021-02-08  7:00   ` Jens Wiklander
2021-02-08  7:46     ` Jorge Ramirez-Ortiz, Foundries
2021-02-08  8:32       ` Jorge Ramirez-Ortiz, Foundries
2021-02-08  8:54         ` Jens Wiklander

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.