Because I did not know about it till right now. Thanks for the
suggestion. Will upload a v2 as soon as I get a chance.

On Wed, Apr 3, 2019 at 12:53 PM Guenter Roeck <gro...@google.com> wrote:
>
> On Wed, Apr 3, 2019 at 12:40 PM Enrico Granata <egran...@google.com> wrote:
> >
> > I can certainly add a "did_print_error" flag or some such, but in practice, 
> > if the transfer function is NULL, the initial handshake will fail, and this 
> > will in turn cause EC registration to fail, and no further communication 
> > should occur, so no further log entries will be printed.
> >
> Sorry, I am a bit lost. Why would dev_err_once() not work ?
>
> Guenter
>
> > Thanks
> >
> > Enrico Granata | egran...@google.com | ChromeOS | MTV1600
> >
> >
> > On Wed, Apr 3, 2019 at 11:51 AM Guenter Roeck <gro...@google.com> wrote:
> >>
> >> On Wed, Apr 3, 2019 at 11:31 AM <egran...@chromium.org> wrote:
> >> >
> >> > From: Enrico Granata <egran...@chromium.org>
> >> >
> >> > As new transfer mechanisms are added to the EC codebase, they may
> >> > not support v2 of the EC protocol.
> >> >
> >> > If the v3 initial handshake transfer fails, the kernel will try
> >> > and call cmd_xfer as a fallback. If v2 is not supported, cmd_xfer
> >> > will be NULL, and the code will end up causing a kernel panic.
> >> >
> >> > Add a check for NULL before calling the transfer function, along
> >> > with a helpful comment explaining how one might end up in this
> >> > situation.
> >> >
> >> > Signed-off-by: Enrico Granata <egran...@chromium.org>
> >> > ---
> >> >  drivers/platform/chrome/cros_ec_proto.c | 10 ++++++++++
> >> >  1 file changed, 10 insertions(+)
> >> >
> >> > diff --git a/drivers/platform/chrome/cros_ec_proto.c 
> >> > b/drivers/platform/chrome/cros_ec_proto.c
> >> > index 97a068dff192d..953076ab401aa 100644
> >> > --- a/drivers/platform/chrome/cros_ec_proto.c
> >> > +++ b/drivers/platform/chrome/cros_ec_proto.c
> >> > @@ -56,6 +56,16 @@ static int send_command(struct cros_ec_device *ec_dev,
> >> >         else
> >> >                 xfer_fxn = ec_dev->cmd_xfer;
> >> >
> >> > +       if (xfer_fxn == NULL) {
> >> > +               /* This error can happen if a communication error 
> >> > happened and
> >> > +                * the EC is trying to use protocol v2, on an underlying
> >> > +                * communication mechanism that does not support v2.
> >> > +                */
> >>
> >> I am not personally a friend of networking-style multi-line comments.
> >>
> >> > +               dev_err(ec_dev->dev,
> >> > +                       "missing EC transfer API, cannot send 
> >> > command\n");
> >>
> >> That message will be displayed each time a message is sent, ie in
> >> practice for each message. Is there any value in that, other than
> >> clogging the log ?
> >>
> >> Guenter
> >>
> >> > +               return -EIO;
> >> > +       }
> >> > +
> >> >         ret = (*xfer_fxn)(ec_dev, msg);
> >> >         if (msg->result == EC_RES_IN_PROGRESS) {
> >> >                 int i;
> >> > --
> >> > 2.21.0.392.gf8f6787159e-goog
> >> >

Reply via email to