On Tue, Jun 12, 2018 at 2:36 PM Tudor Ambarus
<tudor.amba...@microchip.com> wrote:
> On 06/05/2018 04:49 PM, Linus Walleij wrote:

> >       /* send the command */
>
> I guess that this comment will become superfluous if you're going to add
> an error message.

OK stripped obvious comments.

> > -     return atmel_ecc_status(&client->dev, cmd->data);
> > +     ret = atmel_ecc_status(&client->dev, cmd->data);
>
> atmel_ecc_status already prints errors when needed.

OK skipped this change.


> >       ret = atmel_ecc_send_receive(client, cmd);
> > -     if (ret)
> > +     if (ret) {
> > +             dev_err(&client->dev,
> > +                     "failed to send ECC init command\n");
>
> Here we will have two errors reported, the first being from the
> atmel_ecc_send_receive(). I would go with just an error reported. Do we
> really care what happened with the i2c transfer, or it's enough to
> report that the init failed?

The more help the better.

I think it is relevant to have both: you will read the log
and say "OK init failed because this I2C transfer is not
getting across as it should", that is helpful.

Yours,
Linus Walleij

Reply via email to