Hi, Linus,

On 06/28/2018 04:07 PM, Linus Walleij wrote:
> Report errors once when they happen on the I2C bus so we
> get good information in cases such as when the wrong I2C
> address is used.
> 
> Signed-off-by: Linus Walleij <linus.wall...@linaro.org>
> ---
> ChangeLog v1->v2:
> - Strip some comments that are now obvious from the context
>   with the error messages.
> - Do not print the excess ECC error message,  atmel_ecc_status()
>   already prints error messages.
> ---
>  drivers/crypto/atmel-ecc.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> index 33773920e4bf..baef0d07164d 100644
> --- a/drivers/crypto/atmel-ecc.c
> +++ b/drivers/crypto/atmel-ecc.c
> @@ -310,26 +310,31 @@ static int atmel_ecc_send_receive(struct i2c_client 
> *client,
>       mutex_lock(&i2c_priv->lock);
>  
>       ret = atmel_ecc_wakeup(client);
> -     if (ret)
> +     if (ret) {
> +             dev_err(&client->dev, "wakeup failed\n");
>               goto err;
> +     }
>  
> -     /* send the command */
>       ret = i2c_master_send(client, (u8 *)cmd, cmd->count + WORD_ADDR_SIZE);
> -     if (ret < 0)
> +     if (ret < 0) {
> +             dev_err(&client->dev, "command send failed\n");
>               goto err;
> +     }
>  
>       /* delay the appropriate amount of time for command to execute */
>       msleep(cmd->msecs);
>  
> -     /* receive the response */
>       ret = i2c_master_recv(client, cmd->data, cmd->rxsize);
> -     if (ret < 0)
> +     if (ret < 0) {
> +             dev_err(&client->dev, "getting response failed\n");
>               goto err;
> +     }
>  
> -     /* put the device into low-power mode */
>       ret = atmel_ecc_sleep(client);
> -     if (ret < 0)
> +     if (ret < 0) {
> +             dev_err(&client->dev, "putting to sleep failed\n");
>               goto err;
> +     }
>  
>       mutex_unlock(&i2c_priv->lock);
>       return atmel_ecc_status(&client->dev, cmd->data);
> @@ -624,8 +629,11 @@ static int device_sanity_check(struct i2c_client *client)
>       atmel_ecc_init_read_cmd(cmd);
>  
>       ret = atmel_ecc_send_receive(client, cmd);
> -     if (ret)
> +     if (ret) {
> +             dev_err(&client->dev,

If you want to have two errors reported, one related to i2c and the other to
crypto, I propose to be consistent and add an error message for the other call
to atmel_ecc_send_receive() in atmel_ecdh_set_secret().


> +                     "failed to send ECC init command\n");

This line fits well in the previous. I would remove "ECC" from it, looks a bit
redundant:
atmel-ecc 2-0058: failed to send ECC init command

Thanks!
ta

>               goto free_cmd;
> +     }
>  
>       /*
>        * It is vital that the Configuration, Data and OTP zones be locked
> 

Reply via email to