On Thu, Jan 24, 2019 at 07:34:36AM +0100, Andreas Kemnade wrote:
> Some Wi2Wi devices do not have a wakeup output, so device state can
> only be indirectly detected by looking whether there is communication
> over the serial lines.
> This approach requires a report cycle set to a value less than 2 seconds
> to be reliable.
>
> Signed-off-by: Andreas Kemnade <andr...@kemnade.info>
> ---
> Changes in v4:
> - was 3/6 earlier
> - more cleanup
> - separate no wakeup version for sirf_wait_for_power_state
> - cleaned up optimisation for initial power off
>
> Changes in v3:
> - was 2/5 earlier
> - changed commit headline
> - more style cleanup
> - split out initial power off as 2/6
> - introduced SIRF_REPORT_CYCLE constant
> - added documentation about limitations
> - ignore first data after power state on so no
> shutdown meassages are treated as power on success
> - clearer logic in sirf_wait_for_power_state
>
> Changes in v2:
> - style cleanup
> - do not keep serdev open just because runtime is active,
> only when needed (gnss device is opened or state is changed)
> - clearer timeout semantics
>
> drivers/gnss/sirf.c | 124
> ++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 105 insertions(+), 19 deletions(-)
> static int sirf_wait_for_power_state(struct sirf_data *data, bool active,
> unsigned long timeout)
> {
> int ret;
>
> + if (!data->wakeup)
> + return sirf_wait_for_power_state_nowakeup(data, active,
> + timeout);
> +
> ret = wait_event_interruptible_timeout(data->power_wait,
> data->active == active, msecs_to_jiffies(timeout));
> if (ret < 0)
> @@ -195,6 +267,12 @@ static int sirf_set_active(struct sirf_data *data, bool
> active)
> else
> timeout = SIRF_HIBERNATE_TIMEOUT;
>
> + if (!data->wakeup) {
> + ret = sirf_serdev_open(data);
> + if (ret)
> + return ret;
> + }
> +
> do {
> sirf_pulse_on_off(data);
> ret = sirf_wait_for_power_state(data, active, timeout);
> @@ -202,12 +280,17 @@ static int sirf_set_active(struct sirf_data *data, bool
> active)
> if (ret == -ETIMEDOUT)
> continue;
>
> - return ret;
> }
> -
> break;
> +
> } while (retries--);
I noticed there were some odd white-space changes here after you
reverted the modified error handling from v3 which initially looked a
little out of place to me.
I decided to add those changed back in instead after looking at the end
result and noticing that the (retries < 0) check below also becomes
superfluous.
> + if (!data->wakeup)
> + sirf_serdev_close(data);
> +
> + if (ret)
> + return ret;
> +
> if (retries < 0)
> return -ETIMEDOUT;
Series now applied, good job!
Johan
