These patches fixes an issue where the sirf wakeup interrupt was
prematurely enabled during probe. As Andreas suggested, we should also
make sure that the receiver is indeed suspended at probe (to avoid
wasting power until first open or suspend).
Johan
Johan Hovold (3):
gnss: sirf: fix premature wakeup interrupt enable
gnss: sirf: force hibernate mode on probe
gnss: sirf: drop redundant double negation
drivers/gnss/sirf.c | 45 ++++++++++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 13 deletions(-)
--
2.20.1
Make sure the receiver is powered (and booted) before enabling the
wakeup interrupt to avoid spurious interrupts due to a floating input.
Similarly, disable the interrupt before powering off on probe errors and
on unbind.
Fixes: d2efbbd18b1e ("gnss: add driver for sirfstar-based receivers")
Cc: stable <[email protected]> # 4.19
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/gnss/sirf.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/gnss/sirf.c b/drivers/gnss/sirf.c
index 226f6e6fe01b..8e3f6a776e02 100644
--- a/drivers/gnss/sirf.c
+++ b/drivers/gnss/sirf.c
@@ -310,30 +310,26 @@ static int sirf_probe(struct serdev_device *serdev)
ret = -ENODEV;
goto err_put_device;
}
+
+ ret = regulator_enable(data->vcc);
+ if (ret)
+ goto err_put_device;
+
+ /* Wait for chip to boot into hibernate mode. */
+ msleep(SIRF_BOOT_DELAY);
}
if (data->wakeup) {
ret = gpiod_to_irq(data->wakeup);
if (ret < 0)
- goto err_put_device;
-
+ goto err_disable_vcc;
data->irq = ret;
- ret = devm_request_threaded_irq(dev, data->irq, NULL,
- sirf_wakeup_handler,
+ ret = request_threaded_irq(data->irq, NULL, sirf_wakeup_handler,
IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
"wakeup", data);
if (ret)
- goto err_put_device;
- }
-
- if (data->on_off) {
- ret = regulator_enable(data->vcc);
- if (ret)
- goto err_put_device;
-
- /* Wait for chip to boot into hibernate mode */
- msleep(SIRF_BOOT_DELAY);
+ goto err_disable_vcc;
}
if (IS_ENABLED(CONFIG_PM)) {
@@ -342,7 +338,7 @@ static int sirf_probe(struct serdev_device *serdev)
} else {
ret = sirf_runtime_resume(dev);
if (ret < 0)
- goto err_disable_vcc;
+ goto err_free_irq;
}
ret = gnss_register_device(gdev);
@@ -356,6 +352,9 @@ static int sirf_probe(struct serdev_device *serdev)
pm_runtime_disable(dev);
else
sirf_runtime_suspend(dev);
+err_free_irq:
+ if (data->wakeup)
+ free_irq(data->irq, data);
err_disable_vcc:
if (data->on_off)
regulator_disable(data->vcc);
@@ -376,6 +375,9 @@ static void sirf_remove(struct serdev_device *serdev)
else
sirf_runtime_suspend(&serdev->dev);
+ if (data->wakeup)
+ free_irq(data->irq, data);
+
if (data->on_off)
regulator_disable(data->vcc);
--
2.20.1
Make sure to put the receiver in hibernate mode in case it is already
active during probe in order to avoid wasting power until first open or
suspend.
This can happen, for example, after a reset or non-clean shutdown, and
possibly also due to glitches during power-on.
Reported-by: Andreas Kemnade <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/gnss/sirf.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/gnss/sirf.c b/drivers/gnss/sirf.c
index 8e3f6a776e02..f9a9d00dec98 100644
--- a/drivers/gnss/sirf.c
+++ b/drivers/gnss/sirf.c
@@ -320,6 +320,11 @@ static int sirf_probe(struct serdev_device *serdev)
}
if (data->wakeup) {
+ ret = gpiod_get_value_cansleep(data->wakeup);
+ if (ret < 0)
+ goto err_disable_vcc;
+ data->active = ret;
+
ret = gpiod_to_irq(data->wakeup);
if (ret < 0)
goto err_disable_vcc;
@@ -332,6 +337,18 @@ static int sirf_probe(struct serdev_device *serdev)
goto err_disable_vcc;
}
+ if (data->on_off) {
+ /* Force hibernate mode if already active. */
+ if (data->active) {
+ ret = sirf_set_active(data, false);
+ if (ret) {
+ dev_err(dev, "failed to set hibernate mode: %d\n",
+ ret);
+ goto err_free_irq;
+ }
+ }
+ }
+
if (IS_ENABLED(CONFIG_PM)) {
pm_runtime_set_suspended(dev); /* clear runtime_error flag */
pm_runtime_enable(dev);
--
2.20.1
The active flag is of type bool so drop the redundant double negation
when storing the gpio state.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/gnss/sirf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gnss/sirf.c b/drivers/gnss/sirf.c
index f9a9d00dec98..59cde7e923b8 100644
--- a/drivers/gnss/sirf.c
+++ b/drivers/gnss/sirf.c
@@ -125,7 +125,7 @@ static irqreturn_t sirf_wakeup_handler(int irq, void *dev_id)
if (ret < 0)
goto out;
- data->active = !!ret;
+ data->active = ret;
wake_up_interruptible(&data->power_wait);
out:
return IRQ_HANDLED;
--
2.20.1
Hi Johan,
On Tue, 22 Jan 2019 18:22:52 +0100
Johan Hovold <[email protected]> wrote:
> These patches fixes an issue where the sirf wakeup interrupt was
> prematurely enabled during probe. As Andreas suggested, we should also
> make sure that the receiver is indeed suspended at probe (to avoid
> wasting power until first open or suspend).
>
I will rebase my stuff on top of it.
Regards,
Andreas