Re: [PATCH] platform/chrome: wilco_ec: Add debugfs test_event file

[Daniel Campello <campello@chromium.org>]

On Fri, Sep 06, 2019 at 06:20:51PM -0400, Nick Crews wrote:
> Thanks for the patch Daniel! A few thoughts that I didn't
> have on the review on Gerrit, sorry :) After those changes,
>
No problem, I will take care of them.

> Reviewed-by: Nick Crews <ncrews@chromium.org>
>
> >         u8 sub_cmd;     /* Always SUB_CMD_H1_GPIO */
>
> This comment should be removed.
>
I overlooked this. Done.

> This comment should be moved to h1_gpio_get().
>
Agree

> > -static int h1_gpio_get(void *arg, u64 *val)
> > +static int write_to_mailbox(struct wilco_ec_device *ec, u8 sub_cmd, u64 *val)
>
> What about send_ec_cmd() or similar? Something that communicates that
> we are sometimes telling the EC to do something, and sometimes reading something
> back. Also, since we are adding in another layer in here, we can fix
> the signature from
> the one required by a debugfs attribute. Use "ret" instead of "val"
> and make it a u8*.
>
send_ec_cmd() sounds good to me but ret is already used. Will change
to out_val as discussed offline.

> A one line comment as to what test_event does?
>
> > +static int test_event_set(void *arg, u64 val)
> > +{
> > +       u64 ret;
> > +
> > +       return write_to_mailbox(arg, SUB_CMD_TEST_EVENT, &ret);
> > +}
> > +

Will do. Thanks again.