Re: [PATCH v2 5/7] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS

[Lee Jones <lee.jones@linaro.org>]

On Mon, 08 Sep 2014, Javier Martinez Canillas wrote:
> On 09/04/2014 10:34 AM, Lee Jones wrote:
> > On Mon, 25 Aug 2014, Javier Martinez Canillas wrote:
> >> From: Andrew Bresticker <abrestic@chromium.org>
> >> 
> >> When an EC command returns EC_RES_IN_PROGRESS, we need to query
> >> the state of the EC until it indicates that it is no longer busy.
> >> Do this in cros_ec_cmd_xfer() under the EC's mutex so that other
> >> commands (e.g. keyboard, I2C passtru) aren't issued to the EC while
> >> it is working on the in-progress command.
> >> 
> >> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> >> Reviewed-by: Simon Glass <sjg@chromium.org>
> >> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> >> ---
> >> 
> >> Changes since v1:
> >>  - The *xfer() calls don't modify the passed cros_ec_command so there is
> >>    no need to populate it inside the for loop. Suggested by Lee Jones.
> >> ---
> >>  drivers/mfd/cros_ec.c | 34 +++++++++++++++++++++++++++++++++-
> >>  1 file changed, 33 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> >> index c53804a..cd0c93c 100644
> >> --- a/drivers/mfd/cros_ec.c
> >> +++ b/drivers/mfd/cros_ec.c

[...]

> >>  	mutex_lock(&ec_dev->lock);
> >>  	ret = ec_dev->cmd_xfer(ec_dev, msg);
> >> +	if (ret == -EAGAIN && msg->result == EC_RES_IN_PROGRESS) {
> > 
> > Is there ever a time where (ret == -EAGAIN) but (msg->result !=
> > EC_RES_IN_PROGRESS) [note the !=].  And/or is there ever a time where
> > (msg->result == EC_RES_IN_PROGRESS) but (ret != -EAGAIN) [again, not
> > the !=].
> > 
> > Another way of explaining it.  Can ret be anything other than -EAGAIN
> > when the result is EC_RES_IN_PROGRESS.  And can the result be anything
> > other than EC_RES_IN_PROGRESS when ret is -EAGAIN?

[...]

> But after looking at all the cros_ec transport drivers it seems to be safe to
> just check if result is EC_RES_IN_PROGRESS instead of checking also if ret is
> -EAGAIN since (at least on all the current transport drivers) the former
> implies the later.

That's exactly what I was getting at.

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog