* [PATCH RFT V2 0/3] iio: mxs-lradc: implement PM ops
@ 2016-04-21 20:11 ` Stefan Wahren
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-21 20:11 UTC (permalink / raw)
To: Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Marek Vasut
Cc: Fabio Estevam, Juergen Borleis, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Stefan Wahren
This patch series simplifies TS registration and implements suspend/resume
support for mxs-lradc.
Patch 1: simplify TS registration
Patch 2: refactor mxs-lradc in order to prepare PM implementation
Patch 3: implement suspend/resume support
These patches has been tested with i.MX23 and i.MX28 but without
a touchscreen. I added only a bogus touchscreen in devicetree.
It would be nice if someone with a real touchscreen could test it.
Changes in V2:
- rebase on iio/testing since patch 2 and 3 from V1 has been applied
- also remove input_unregister_device in patch 1
- use mask of virtual channels for IRQ disabling in patch 2
Stefan Wahren (3):
iio: mxs-lradc: simplify TS registration
iio: mxs-lradc: disable only masked channels in mxs_lradc_hw_stop
iio: mxs-lradc: implement suspend/resume support
drivers/iio/adc/mxs-lradc.c | 81 +++++++++++++++++++++++++++++++++++--------
1 file changed, 66 insertions(+), 15 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 0/3] iio: mxs-lradc: implement PM ops
@ 2016-04-21 20:11 ` Stefan Wahren
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-21 20:11 UTC (permalink / raw)
To: Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Marek Vasut
Cc: Fabio Estevam, Juergen Borleis, linux-iio, linux-input,
linux-arm-kernel, Stefan Wahren
This patch series simplifies TS registration and implements suspend/resume
support for mxs-lradc.
Patch 1: simplify TS registration
Patch 2: refactor mxs-lradc in order to prepare PM implementation
Patch 3: implement suspend/resume support
These patches has been tested with i.MX23 and i.MX28 but without
a touchscreen. I added only a bogus touchscreen in devicetree.
It would be nice if someone with a real touchscreen could test it.
Changes in V2:
- rebase on iio/testing since patch 2 and 3 from V1 has been applied
- also remove input_unregister_device in patch 1
- use mask of virtual channels for IRQ disabling in patch 2
Stefan Wahren (3):
iio: mxs-lradc: simplify TS registration
iio: mxs-lradc: disable only masked channels in mxs_lradc_hw_stop
iio: mxs-lradc: implement suspend/resume support
drivers/iio/adc/mxs-lradc.c | 81 +++++++++++++++++++++++++++++++++++--------
1 file changed, 66 insertions(+), 15 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 0/3] iio: mxs-lradc: implement PM ops
@ 2016-04-21 20:11 ` Stefan Wahren
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-21 20:11 UTC (permalink / raw)
To: linux-arm-kernel
This patch series simplifies TS registration and implements suspend/resume
support for mxs-lradc.
Patch 1: simplify TS registration
Patch 2: refactor mxs-lradc in order to prepare PM implementation
Patch 3: implement suspend/resume support
These patches has been tested with i.MX23 and i.MX28 but without
a touchscreen. I added only a bogus touchscreen in devicetree.
It would be nice if someone with a real touchscreen could test it.
Changes in V2:
- rebase on iio/testing since patch 2 and 3 from V1 has been applied
- also remove input_unregister_device in patch 1
- use mask of virtual channels for IRQ disabling in patch 2
Stefan Wahren (3):
iio: mxs-lradc: simplify TS registration
iio: mxs-lradc: disable only masked channels in mxs_lradc_hw_stop
iio: mxs-lradc: implement suspend/resume support
drivers/iio/adc/mxs-lradc.c | 81 +++++++++++++++++++++++++++++++++++--------
1 file changed, 66 insertions(+), 15 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 1/3] iio: mxs-lradc: simplify TS registration
2016-04-21 20:11 ` Stefan Wahren
@ 2016-04-21 20:11 ` Stefan Wahren
-1 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-21 20:11 UTC (permalink / raw)
To: Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Marek Vasut
Cc: Fabio Estevam, Juergen Borleis, linux-iio, linux-input,
linux-arm-kernel, Stefan Wahren
This patch simplifies the TS registration of mxs-lradc by
using devm_input_allocate_device.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
drivers/iio/adc/mxs-lradc.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
index ad26da1..223650b 100644
--- a/drivers/iio/adc/mxs-lradc.c
+++ b/drivers/iio/adc/mxs-lradc.c
@@ -1120,12 +1120,11 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
{
struct input_dev *input;
struct device *dev = lradc->dev;
- int ret;
if (!lradc->use_touchscreen)
return 0;
- input = input_allocate_device();
+ input = devm_input_allocate_device(dev);
if (!input)
return -ENOMEM;
@@ -1146,11 +1145,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
lradc->ts_input = input;
input_set_drvdata(input, lradc);
- ret = input_register_device(input);
- if (ret)
- input_free_device(lradc->ts_input);
- return ret;
+ return input_register_device(input);
}
static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
@@ -1159,7 +1155,6 @@ static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
return;
mxs_lradc_disable_ts(lradc);
- input_unregister_device(lradc->ts_input);
}
/*
--
1.7.9.5
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH RFT V2 1/3] iio: mxs-lradc: simplify TS registration
@ 2016-04-21 20:11 ` Stefan Wahren
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-21 20:11 UTC (permalink / raw)
To: linux-arm-kernel
This patch simplifies the TS registration of mxs-lradc by
using devm_input_allocate_device.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
drivers/iio/adc/mxs-lradc.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
index ad26da1..223650b 100644
--- a/drivers/iio/adc/mxs-lradc.c
+++ b/drivers/iio/adc/mxs-lradc.c
@@ -1120,12 +1120,11 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
{
struct input_dev *input;
struct device *dev = lradc->dev;
- int ret;
if (!lradc->use_touchscreen)
return 0;
- input = input_allocate_device();
+ input = devm_input_allocate_device(dev);
if (!input)
return -ENOMEM;
@@ -1146,11 +1145,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
lradc->ts_input = input;
input_set_drvdata(input, lradc);
- ret = input_register_device(input);
- if (ret)
- input_free_device(lradc->ts_input);
- return ret;
+ return input_register_device(input);
}
static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
@@ -1159,7 +1155,6 @@ static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
return;
mxs_lradc_disable_ts(lradc);
- input_unregister_device(lradc->ts_input);
}
/*
--
1.7.9.5
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH RFT V2 2/3] iio: mxs-lradc: disable only masked channels in mxs_lradc_hw_stop
2016-04-21 20:11 ` Stefan Wahren
(?)
@ 2016-04-21 20:11 ` Stefan Wahren
-1 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-21 20:11 UTC (permalink / raw)
To: Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Marek Vasut
Cc: Fabio Estevam, Juergen Borleis, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Stefan Wahren
Disabling of the touchscreen IRQs should be done in
mxs_lradc_disable_ts. So disable only the masked virtual channels
in mxs_lradc_hw_stop and finally remove the unused function
mxs_lradc_irq_en_mask.
Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
---
drivers/iio/adc/mxs-lradc.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
index 223650b..cfc558f 100644
--- a/drivers/iio/adc/mxs-lradc.c
+++ b/drivers/iio/adc/mxs-lradc.c
@@ -373,13 +373,6 @@ static u32 mxs_lradc_plate_mask(struct mxs_lradc *lradc)
return LRADC_CTRL0_MX28_PLATE_MASK;
}
-static u32 mxs_lradc_irq_en_mask(struct mxs_lradc *lradc)
-{
- if (lradc->soc == IMX23_LRADC)
- return LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK;
- return LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK;
-}
-
static u32 mxs_lradc_irq_mask(struct mxs_lradc *lradc)
{
if (lradc->soc == IMX23_LRADC)
@@ -1505,7 +1498,9 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
{
int i;
- mxs_lradc_reg_clear(lradc, mxs_lradc_irq_en_mask(lradc), LRADC_CTRL1);
+ mxs_lradc_reg_clear(lradc,
+ lradc->buffer_vchans << LRADC_CTRL1_LRADC_IRQ_EN_OFFSET,
+ LRADC_CTRL1);
for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
mxs_lradc_reg_wrt(lradc, 0, LRADC_DELAY(i));
--
1.7.9.5
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH RFT V2 2/3] iio: mxs-lradc: disable only masked channels in mxs_lradc_hw_stop
@ 2016-04-21 20:11 ` Stefan Wahren
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-21 20:11 UTC (permalink / raw)
To: Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Marek Vasut
Cc: Fabio Estevam, Juergen Borleis, linux-iio, linux-input,
linux-arm-kernel, Stefan Wahren
Disabling of the touchscreen IRQs should be done in
mxs_lradc_disable_ts. So disable only the masked virtual channels
in mxs_lradc_hw_stop and finally remove the unused function
mxs_lradc_irq_en_mask.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
drivers/iio/adc/mxs-lradc.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
index 223650b..cfc558f 100644
--- a/drivers/iio/adc/mxs-lradc.c
+++ b/drivers/iio/adc/mxs-lradc.c
@@ -373,13 +373,6 @@ static u32 mxs_lradc_plate_mask(struct mxs_lradc *lradc)
return LRADC_CTRL0_MX28_PLATE_MASK;
}
-static u32 mxs_lradc_irq_en_mask(struct mxs_lradc *lradc)
-{
- if (lradc->soc == IMX23_LRADC)
- return LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK;
- return LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK;
-}
-
static u32 mxs_lradc_irq_mask(struct mxs_lradc *lradc)
{
if (lradc->soc == IMX23_LRADC)
@@ -1505,7 +1498,9 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
{
int i;
- mxs_lradc_reg_clear(lradc, mxs_lradc_irq_en_mask(lradc), LRADC_CTRL1);
+ mxs_lradc_reg_clear(lradc,
+ lradc->buffer_vchans << LRADC_CTRL1_LRADC_IRQ_EN_OFFSET,
+ LRADC_CTRL1);
for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
mxs_lradc_reg_wrt(lradc, 0, LRADC_DELAY(i));
--
1.7.9.5
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH RFT V2 2/3] iio: mxs-lradc: disable only masked channels in mxs_lradc_hw_stop
@ 2016-04-21 20:11 ` Stefan Wahren
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-21 20:11 UTC (permalink / raw)
To: linux-arm-kernel
Disabling of the touchscreen IRQs should be done in
mxs_lradc_disable_ts. So disable only the masked virtual channels
in mxs_lradc_hw_stop and finally remove the unused function
mxs_lradc_irq_en_mask.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
drivers/iio/adc/mxs-lradc.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
index 223650b..cfc558f 100644
--- a/drivers/iio/adc/mxs-lradc.c
+++ b/drivers/iio/adc/mxs-lradc.c
@@ -373,13 +373,6 @@ static u32 mxs_lradc_plate_mask(struct mxs_lradc *lradc)
return LRADC_CTRL0_MX28_PLATE_MASK;
}
-static u32 mxs_lradc_irq_en_mask(struct mxs_lradc *lradc)
-{
- if (lradc->soc == IMX23_LRADC)
- return LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK;
- return LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK;
-}
-
static u32 mxs_lradc_irq_mask(struct mxs_lradc *lradc)
{
if (lradc->soc == IMX23_LRADC)
@@ -1505,7 +1498,9 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
{
int i;
- mxs_lradc_reg_clear(lradc, mxs_lradc_irq_en_mask(lradc), LRADC_CTRL1);
+ mxs_lradc_reg_clear(lradc,
+ lradc->buffer_vchans << LRADC_CTRL1_LRADC_IRQ_EN_OFFSET,
+ LRADC_CTRL1);
for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
mxs_lradc_reg_wrt(lradc, 0, LRADC_DELAY(i));
--
1.7.9.5
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
2016-04-21 20:11 ` Stefan Wahren
@ 2016-04-21 20:11 ` Stefan Wahren
-1 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-21 20:11 UTC (permalink / raw)
To: Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Marek Vasut
Cc: Fabio Estevam, Juergen Borleis, linux-iio, linux-input,
linux-arm-kernel, Stefan Wahren
This patch implements suspend/resume support for mxs-lradc.
It's possible to use the touchscreen as wakeup source.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
index cfc558f..6bd387f 100644
--- a/drivers/iio/adc/mxs-lradc.c
+++ b/drivers/iio/adc/mxs-lradc.c
@@ -29,6 +29,7 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
+#include <linux/pm.h>
#include <linux/slab.h>
#include <linux/stmp_device.h>
#include <linux/sysfs.h>
@@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
return 0;
}
+static int __maybe_unused mxs_lradc_suspend(struct device *dev)
+{
+ struct iio_dev *iio = dev_get_drvdata(dev);
+ struct mxs_lradc *lradc = iio_priv(iio);
+ struct input_dev *input = lradc->ts_input;
+ int ret = 0;
+
+ if (input) {
+ mutex_lock(&input->mutex);
+
+ /* Enable touchscreen wakeup irq */
+ if (input->users && device_may_wakeup(dev))
+ ret = enable_irq_wake(lradc->irq[0]);
+ else
+ mxs_lradc_disable_ts(lradc);
+
+ mutex_unlock(&input->mutex);
+ }
+
+ if (ret)
+ return ret;
+
+ mxs_lradc_hw_stop(lradc);
+
+ clk_disable_unprepare(lradc->clk);
+
+ return ret;
+}
+
+static int __maybe_unused mxs_lradc_resume(struct device *dev)
+{
+ struct iio_dev *iio = dev_get_drvdata(dev);
+ struct mxs_lradc *lradc = iio_priv(iio);
+ struct input_dev *input = lradc->ts_input;
+ int ret;
+
+ ret = clk_prepare_enable(lradc->clk);
+ if (ret)
+ return ret;
+
+ mxs_lradc_hw_init(lradc);
+
+ if (input) {
+ mutex_lock(&input->mutex);
+
+ /* Disable touchscreen wakeup irq */
+ if (input->users && device_may_wakeup(dev))
+ ret = disable_irq_wake(lradc->irq[0]);
+ else
+ mxs_lradc_enable_touch_detection(lradc);
+
+ mutex_unlock(&input->mutex);
+ }
+
+ return ret;
+}
+
+static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
+
static struct platform_driver mxs_lradc_driver = {
.driver = {
.name = DRIVER_NAME,
.of_match_table = mxs_lradc_dt_ids,
+ .pm = &mxs_lradc_pm_ops,
},
.probe = mxs_lradc_probe,
.remove = mxs_lradc_remove,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-21 20:11 ` Stefan Wahren
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-21 20:11 UTC (permalink / raw)
To: linux-arm-kernel
This patch implements suspend/resume support for mxs-lradc.
It's possible to use the touchscreen as wakeup source.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
index cfc558f..6bd387f 100644
--- a/drivers/iio/adc/mxs-lradc.c
+++ b/drivers/iio/adc/mxs-lradc.c
@@ -29,6 +29,7 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
+#include <linux/pm.h>
#include <linux/slab.h>
#include <linux/stmp_device.h>
#include <linux/sysfs.h>
@@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
return 0;
}
+static int __maybe_unused mxs_lradc_suspend(struct device *dev)
+{
+ struct iio_dev *iio = dev_get_drvdata(dev);
+ struct mxs_lradc *lradc = iio_priv(iio);
+ struct input_dev *input = lradc->ts_input;
+ int ret = 0;
+
+ if (input) {
+ mutex_lock(&input->mutex);
+
+ /* Enable touchscreen wakeup irq */
+ if (input->users && device_may_wakeup(dev))
+ ret = enable_irq_wake(lradc->irq[0]);
+ else
+ mxs_lradc_disable_ts(lradc);
+
+ mutex_unlock(&input->mutex);
+ }
+
+ if (ret)
+ return ret;
+
+ mxs_lradc_hw_stop(lradc);
+
+ clk_disable_unprepare(lradc->clk);
+
+ return ret;
+}
+
+static int __maybe_unused mxs_lradc_resume(struct device *dev)
+{
+ struct iio_dev *iio = dev_get_drvdata(dev);
+ struct mxs_lradc *lradc = iio_priv(iio);
+ struct input_dev *input = lradc->ts_input;
+ int ret;
+
+ ret = clk_prepare_enable(lradc->clk);
+ if (ret)
+ return ret;
+
+ mxs_lradc_hw_init(lradc);
+
+ if (input) {
+ mutex_lock(&input->mutex);
+
+ /* Disable touchscreen wakeup irq */
+ if (input->users && device_may_wakeup(dev))
+ ret = disable_irq_wake(lradc->irq[0]);
+ else
+ mxs_lradc_enable_touch_detection(lradc);
+
+ mutex_unlock(&input->mutex);
+ }
+
+ return ret;
+}
+
+static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
+
static struct platform_driver mxs_lradc_driver = {
.driver = {
.name = DRIVER_NAME,
.of_match_table = mxs_lradc_dt_ids,
+ .pm = &mxs_lradc_pm_ops,
},
.probe = mxs_lradc_probe,
.remove = mxs_lradc_remove,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 1/3] iio: mxs-lradc: simplify TS registration
2016-04-21 20:11 ` Stefan Wahren
(?)
@ 2016-04-21 20:39 ` Marek Vasut
-1 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2016-04-21 20:39 UTC (permalink / raw)
To: Stefan Wahren, Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: Fabio Estevam, Juergen Borleis, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 04/21/2016 10:11 PM, Stefan Wahren wrote:
> This patch simplifies the TS registration of mxs-lradc by
> using devm_input_allocate_device.
>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
> drivers/iio/adc/mxs-lradc.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index ad26da1..223650b 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -1120,12 +1120,11 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> {
> struct input_dev *input;
> struct device *dev = lradc->dev;
> - int ret;
>
> if (!lradc->use_touchscreen)
> return 0;
>
> - input = input_allocate_device();
> + input = devm_input_allocate_device(dev);
> if (!input)
> return -ENOMEM;
>
> @@ -1146,11 +1145,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
>
> lradc->ts_input = input;
> input_set_drvdata(input, lradc);
> - ret = input_register_device(input);
> - if (ret)
> - input_free_device(lradc->ts_input);
>
> - return ret;
> + return input_register_device(input);
> }
>
> static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> @@ -1159,7 +1155,6 @@ static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> return;
>
> mxs_lradc_disable_ts(lradc);
> - input_unregister_device(lradc->ts_input);
> }
>
> /*
>
This seems to be done right now :)
Reviewed-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 1/3] iio: mxs-lradc: simplify TS registration
@ 2016-04-21 20:39 ` Marek Vasut
0 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2016-04-21 20:39 UTC (permalink / raw)
To: Stefan Wahren, Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: Fabio Estevam, Juergen Borleis, linux-iio, linux-input, linux-arm-kernel
On 04/21/2016 10:11 PM, Stefan Wahren wrote:
> This patch simplifies the TS registration of mxs-lradc by
> using devm_input_allocate_device.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/iio/adc/mxs-lradc.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index ad26da1..223650b 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -1120,12 +1120,11 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> {
> struct input_dev *input;
> struct device *dev = lradc->dev;
> - int ret;
>
> if (!lradc->use_touchscreen)
> return 0;
>
> - input = input_allocate_device();
> + input = devm_input_allocate_device(dev);
> if (!input)
> return -ENOMEM;
>
> @@ -1146,11 +1145,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
>
> lradc->ts_input = input;
> input_set_drvdata(input, lradc);
> - ret = input_register_device(input);
> - if (ret)
> - input_free_device(lradc->ts_input);
>
> - return ret;
> + return input_register_device(input);
> }
>
> static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> @@ -1159,7 +1155,6 @@ static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> return;
>
> mxs_lradc_disable_ts(lradc);
> - input_unregister_device(lradc->ts_input);
> }
>
> /*
>
This seems to be done right now :)
Reviewed-by: Marek Vasut <marex@denx.de>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 1/3] iio: mxs-lradc: simplify TS registration
@ 2016-04-21 20:39 ` Marek Vasut
0 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2016-04-21 20:39 UTC (permalink / raw)
To: linux-arm-kernel
On 04/21/2016 10:11 PM, Stefan Wahren wrote:
> This patch simplifies the TS registration of mxs-lradc by
> using devm_input_allocate_device.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/iio/adc/mxs-lradc.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index ad26da1..223650b 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -1120,12 +1120,11 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> {
> struct input_dev *input;
> struct device *dev = lradc->dev;
> - int ret;
>
> if (!lradc->use_touchscreen)
> return 0;
>
> - input = input_allocate_device();
> + input = devm_input_allocate_device(dev);
> if (!input)
> return -ENOMEM;
>
> @@ -1146,11 +1145,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
>
> lradc->ts_input = input;
> input_set_drvdata(input, lradc);
> - ret = input_register_device(input);
> - if (ret)
> - input_free_device(lradc->ts_input);
>
> - return ret;
> + return input_register_device(input);
> }
>
> static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> @@ -1159,7 +1155,6 @@ static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> return;
>
> mxs_lradc_disable_ts(lradc);
> - input_unregister_device(lradc->ts_input);
> }
>
> /*
>
This seems to be done right now :)
Reviewed-by: Marek Vasut <marex@denx.de>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 2/3] iio: mxs-lradc: disable only masked channels in mxs_lradc_hw_stop
2016-04-21 20:11 ` Stefan Wahren
(?)
@ 2016-04-21 20:44 ` Marek Vasut
-1 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2016-04-21 20:44 UTC (permalink / raw)
To: Stefan Wahren, Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: Fabio Estevam, Juergen Borleis, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 04/21/2016 10:11 PM, Stefan Wahren wrote:
> Disabling of the touchscreen IRQs should be done in
> mxs_lradc_disable_ts. So disable only the masked virtual channels
> in mxs_lradc_hw_stop and finally remove the unused function
> mxs_lradc_irq_en_mask.
>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
> drivers/iio/adc/mxs-lradc.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index 223650b..cfc558f 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -373,13 +373,6 @@ static u32 mxs_lradc_plate_mask(struct mxs_lradc *lradc)
> return LRADC_CTRL0_MX28_PLATE_MASK;
> }
>
> -static u32 mxs_lradc_irq_en_mask(struct mxs_lradc *lradc)
> -{
> - if (lradc->soc == IMX23_LRADC)
> - return LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK;
> - return LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK;
> -}
> -
> static u32 mxs_lradc_irq_mask(struct mxs_lradc *lradc)
> {
> if (lradc->soc == IMX23_LRADC)
> @@ -1505,7 +1498,9 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
> {
> int i;
>
> - mxs_lradc_reg_clear(lradc, mxs_lradc_irq_en_mask(lradc), LRADC_CTRL1);
> + mxs_lradc_reg_clear(lradc,
> + lradc->buffer_vchans << LRADC_CTRL1_LRADC_IRQ_EN_OFFSET,
> + LRADC_CTRL1);
>
> for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
> mxs_lradc_reg_wrt(lradc, 0, LRADC_DELAY(i));
>
Looks sane too:
Reviewed-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 2/3] iio: mxs-lradc: disable only masked channels in mxs_lradc_hw_stop
@ 2016-04-21 20:44 ` Marek Vasut
0 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2016-04-21 20:44 UTC (permalink / raw)
To: Stefan Wahren, Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: Fabio Estevam, Juergen Borleis, linux-iio, linux-input, linux-arm-kernel
On 04/21/2016 10:11 PM, Stefan Wahren wrote:
> Disabling of the touchscreen IRQs should be done in
> mxs_lradc_disable_ts. So disable only the masked virtual channels
> in mxs_lradc_hw_stop and finally remove the unused function
> mxs_lradc_irq_en_mask.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/iio/adc/mxs-lradc.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index 223650b..cfc558f 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -373,13 +373,6 @@ static u32 mxs_lradc_plate_mask(struct mxs_lradc *lradc)
> return LRADC_CTRL0_MX28_PLATE_MASK;
> }
>
> -static u32 mxs_lradc_irq_en_mask(struct mxs_lradc *lradc)
> -{
> - if (lradc->soc == IMX23_LRADC)
> - return LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK;
> - return LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK;
> -}
> -
> static u32 mxs_lradc_irq_mask(struct mxs_lradc *lradc)
> {
> if (lradc->soc == IMX23_LRADC)
> @@ -1505,7 +1498,9 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
> {
> int i;
>
> - mxs_lradc_reg_clear(lradc, mxs_lradc_irq_en_mask(lradc), LRADC_CTRL1);
> + mxs_lradc_reg_clear(lradc,
> + lradc->buffer_vchans << LRADC_CTRL1_LRADC_IRQ_EN_OFFSET,
> + LRADC_CTRL1);
>
> for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
> mxs_lradc_reg_wrt(lradc, 0, LRADC_DELAY(i));
>
Looks sane too:
Reviewed-by: Marek Vasut <marex@denx.de>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 2/3] iio: mxs-lradc: disable only masked channels in mxs_lradc_hw_stop
@ 2016-04-21 20:44 ` Marek Vasut
0 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2016-04-21 20:44 UTC (permalink / raw)
To: linux-arm-kernel
On 04/21/2016 10:11 PM, Stefan Wahren wrote:
> Disabling of the touchscreen IRQs should be done in
> mxs_lradc_disable_ts. So disable only the masked virtual channels
> in mxs_lradc_hw_stop and finally remove the unused function
> mxs_lradc_irq_en_mask.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/iio/adc/mxs-lradc.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index 223650b..cfc558f 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -373,13 +373,6 @@ static u32 mxs_lradc_plate_mask(struct mxs_lradc *lradc)
> return LRADC_CTRL0_MX28_PLATE_MASK;
> }
>
> -static u32 mxs_lradc_irq_en_mask(struct mxs_lradc *lradc)
> -{
> - if (lradc->soc == IMX23_LRADC)
> - return LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK;
> - return LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK;
> -}
> -
> static u32 mxs_lradc_irq_mask(struct mxs_lradc *lradc)
> {
> if (lradc->soc == IMX23_LRADC)
> @@ -1505,7 +1498,9 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)
> {
> int i;
>
> - mxs_lradc_reg_clear(lradc, mxs_lradc_irq_en_mask(lradc), LRADC_CTRL1);
> + mxs_lradc_reg_clear(lradc,
> + lradc->buffer_vchans << LRADC_CTRL1_LRADC_IRQ_EN_OFFSET,
> + LRADC_CTRL1);
>
> for (i = 0; i < LRADC_MAX_DELAY_CHANS; i++)
> mxs_lradc_reg_wrt(lradc, 0, LRADC_DELAY(i));
>
Looks sane too:
Reviewed-by: Marek Vasut <marex@denx.de>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
2016-04-21 20:11 ` Stefan Wahren
(?)
@ 2016-04-21 20:45 ` Marek Vasut
-1 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2016-04-21 20:45 UTC (permalink / raw)
To: Stefan Wahren, Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: Fabio Estevam, Juergen Borleis, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 04/21/2016 10:11 PM, Stefan Wahren wrote:
> This patch implements suspend/resume support for mxs-lradc.
> It's possible to use the touchscreen as wakeup source.
>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index cfc558f..6bd387f 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -29,6 +29,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/pm.h>
> #include <linux/slab.h>
> #include <linux/stmp_device.h>
> #include <linux/sysfs.h>
> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
Can lradc->ts_input contain some non-valid pointer ? You should make
sure it's either inited to NULL or valid pointer, which I'm not sure
you do.
> + int ret = 0;
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Enable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = enable_irq_wake(lradc->irq[0]);
> + else
> + mxs_lradc_disable_ts(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + if (ret)
> + return ret;
> +
> + mxs_lradc_hw_stop(lradc);
> +
> + clk_disable_unprepare(lradc->clk);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused mxs_lradc_resume(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
> + int ret;
> +
> + ret = clk_prepare_enable(lradc->clk);
> + if (ret)
> + return ret;
> +
> + mxs_lradc_hw_init(lradc);
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Disable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = disable_irq_wake(lradc->irq[0]);
> + else
> + mxs_lradc_enable_touch_detection(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
> +
> static struct platform_driver mxs_lradc_driver = {
> .driver = {
> .name = DRIVER_NAME,
> .of_match_table = mxs_lradc_dt_ids,
> + .pm = &mxs_lradc_pm_ops,
> },
> .probe = mxs_lradc_probe,
> .remove = mxs_lradc_remove,
>
Looks OK otherwise:
Reviewed-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-21 20:45 ` Marek Vasut
0 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2016-04-21 20:45 UTC (permalink / raw)
To: Stefan Wahren, Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: Fabio Estevam, Juergen Borleis, linux-iio, linux-input, linux-arm-kernel
On 04/21/2016 10:11 PM, Stefan Wahren wrote:
> This patch implements suspend/resume support for mxs-lradc.
> It's possible to use the touchscreen as wakeup source.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index cfc558f..6bd387f 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -29,6 +29,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/pm.h>
> #include <linux/slab.h>
> #include <linux/stmp_device.h>
> #include <linux/sysfs.h>
> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
Can lradc->ts_input contain some non-valid pointer ? You should make
sure it's either inited to NULL or valid pointer, which I'm not sure
you do.
> + int ret = 0;
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Enable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = enable_irq_wake(lradc->irq[0]);
> + else
> + mxs_lradc_disable_ts(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + if (ret)
> + return ret;
> +
> + mxs_lradc_hw_stop(lradc);
> +
> + clk_disable_unprepare(lradc->clk);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused mxs_lradc_resume(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
> + int ret;
> +
> + ret = clk_prepare_enable(lradc->clk);
> + if (ret)
> + return ret;
> +
> + mxs_lradc_hw_init(lradc);
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Disable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = disable_irq_wake(lradc->irq[0]);
> + else
> + mxs_lradc_enable_touch_detection(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
> +
> static struct platform_driver mxs_lradc_driver = {
> .driver = {
> .name = DRIVER_NAME,
> .of_match_table = mxs_lradc_dt_ids,
> + .pm = &mxs_lradc_pm_ops,
> },
> .probe = mxs_lradc_probe,
> .remove = mxs_lradc_remove,
>
Looks OK otherwise:
Reviewed-by: Marek Vasut <marex@denx.de>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-21 20:45 ` Marek Vasut
0 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2016-04-21 20:45 UTC (permalink / raw)
To: linux-arm-kernel
On 04/21/2016 10:11 PM, Stefan Wahren wrote:
> This patch implements suspend/resume support for mxs-lradc.
> It's possible to use the touchscreen as wakeup source.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index cfc558f..6bd387f 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -29,6 +29,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/pm.h>
> #include <linux/slab.h>
> #include <linux/stmp_device.h>
> #include <linux/sysfs.h>
> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
Can lradc->ts_input contain some non-valid pointer ? You should make
sure it's either inited to NULL or valid pointer, which I'm not sure
you do.
> + int ret = 0;
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Enable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = enable_irq_wake(lradc->irq[0]);
> + else
> + mxs_lradc_disable_ts(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + if (ret)
> + return ret;
> +
> + mxs_lradc_hw_stop(lradc);
> +
> + clk_disable_unprepare(lradc->clk);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused mxs_lradc_resume(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
> + int ret;
> +
> + ret = clk_prepare_enable(lradc->clk);
> + if (ret)
> + return ret;
> +
> + mxs_lradc_hw_init(lradc);
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Disable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = disable_irq_wake(lradc->irq[0]);
> + else
> + mxs_lradc_enable_touch_detection(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
> +
> static struct platform_driver mxs_lradc_driver = {
> .driver = {
> .name = DRIVER_NAME,
> .of_match_table = mxs_lradc_dt_ids,
> + .pm = &mxs_lradc_pm_ops,
> },
> .probe = mxs_lradc_probe,
> .remove = mxs_lradc_remove,
>
Looks OK otherwise:
Reviewed-by: Marek Vasut <marex@denx.de>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 1/3] iio: mxs-lradc: simplify TS registration
2016-04-21 20:11 ` Stefan Wahren
@ 2016-04-21 21:06 ` Dmitry Torokhov
-1 siblings, 0 replies; 35+ messages in thread
From: Dmitry Torokhov @ 2016-04-21 21:06 UTC (permalink / raw)
To: Stefan Wahren
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Marek Vasut, Fabio Estevam,
Juergen Borleis, linux-iio, linux-input, linux-arm-kernel
On Thu, Apr 21, 2016 at 08:11:16PM +0000, Stefan Wahren wrote:
> This patch simplifies the TS registration of mxs-lradc by
> using devm_input_allocate_device.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/iio/adc/mxs-lradc.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index ad26da1..223650b 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -1120,12 +1120,11 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> {
> struct input_dev *input;
> struct device *dev = lradc->dev;
> - int ret;
>
> if (!lradc->use_touchscreen)
> return 0;
>
> - input = input_allocate_device();
> + input = devm_input_allocate_device(dev);
Also please drop "input->dev.parent = dev;" below.
> if (!input)
> return -ENOMEM;
>
> @@ -1146,11 +1145,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
>
> lradc->ts_input = input;
> input_set_drvdata(input, lradc);
> - ret = input_register_device(input);
> - if (ret)
> - input_free_device(lradc->ts_input);
>
> - return ret;
> + return input_register_device(input);
> }
>
> static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> @@ -1159,7 +1155,6 @@ static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> return;
>
> mxs_lradc_disable_ts(lradc);
I do not think this is needed, since mxs_lradc_ts_close() does this for
us (and it will be called automatically if input device is open at
unregistration time), and if you remove this you can remove
mxs_lradc_ts_unregister() altogether.
> - input_unregister_device(lradc->ts_input);
> }
>
> /*
> --
> 1.7.9.5
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 1/3] iio: mxs-lradc: simplify TS registration
@ 2016-04-21 21:06 ` Dmitry Torokhov
0 siblings, 0 replies; 35+ messages in thread
From: Dmitry Torokhov @ 2016-04-21 21:06 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 21, 2016 at 08:11:16PM +0000, Stefan Wahren wrote:
> This patch simplifies the TS registration of mxs-lradc by
> using devm_input_allocate_device.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/iio/adc/mxs-lradc.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index ad26da1..223650b 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -1120,12 +1120,11 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
> {
> struct input_dev *input;
> struct device *dev = lradc->dev;
> - int ret;
>
> if (!lradc->use_touchscreen)
> return 0;
>
> - input = input_allocate_device();
> + input = devm_input_allocate_device(dev);
Also please drop "input->dev.parent = dev;" below.
> if (!input)
> return -ENOMEM;
>
> @@ -1146,11 +1145,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc)
>
> lradc->ts_input = input;
> input_set_drvdata(input, lradc);
> - ret = input_register_device(input);
> - if (ret)
> - input_free_device(lradc->ts_input);
>
> - return ret;
> + return input_register_device(input);
> }
>
> static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> @@ -1159,7 +1155,6 @@ static void mxs_lradc_ts_unregister(struct mxs_lradc *lradc)
> return;
>
> mxs_lradc_disable_ts(lradc);
I do not think this is needed, since mxs_lradc_ts_close() does this for
us (and it will be called automatically if input device is open at
unregistration time), and if you remove this you can remove
mxs_lradc_ts_unregister() altogether.
> - input_unregister_device(lradc->ts_input);
> }
>
> /*
> --
> 1.7.9.5
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
2016-04-21 20:11 ` Stefan Wahren
(?)
@ 2016-04-21 21:08 ` Dmitry Torokhov
-1 siblings, 0 replies; 35+ messages in thread
From: Dmitry Torokhov @ 2016-04-21 21:08 UTC (permalink / raw)
To: Stefan Wahren
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Marek Vasut, Fabio Estevam,
Juergen Borleis, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Thu, Apr 21, 2016 at 08:11:18PM +0000, Stefan Wahren wrote:
> This patch implements suspend/resume support for mxs-lradc.
> It's possible to use the touchscreen as wakeup source.
>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index cfc558f..6bd387f 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -29,6 +29,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/pm.h>
> #include <linux/slab.h>
> #include <linux/stmp_device.h>
> #include <linux/sysfs.h>
> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
> + int ret = 0;
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Enable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = enable_irq_wake(lradc->irq[0]);
> + else
> + mxs_lradc_disable_ts(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + if (ret)
> + return ret;
I'd rather we had it right after we do:
ret = enable_irq_wake(lradc->irq[0]);
> +
> + mxs_lradc_hw_stop(lradc);
> +
> + clk_disable_unprepare(lradc->clk);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused mxs_lradc_resume(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
> + int ret;
> +
> + ret = clk_prepare_enable(lradc->clk);
> + if (ret)
> + return ret;
> +
> + mxs_lradc_hw_init(lradc);
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Disable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = disable_irq_wake(lradc->irq[0]);
Do we need to fail resume if this call fails?
> + else
> + mxs_lradc_enable_touch_detection(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
> +
> static struct platform_driver mxs_lradc_driver = {
> .driver = {
> .name = DRIVER_NAME,
> .of_match_table = mxs_lradc_dt_ids,
> + .pm = &mxs_lradc_pm_ops,
> },
> .probe = mxs_lradc_probe,
> .remove = mxs_lradc_remove,
> --
> 1.7.9.5
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-21 21:08 ` Dmitry Torokhov
0 siblings, 0 replies; 35+ messages in thread
From: Dmitry Torokhov @ 2016-04-21 21:08 UTC (permalink / raw)
To: Stefan Wahren
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Marek Vasut, Fabio Estevam,
Juergen Borleis, linux-iio, linux-input, linux-arm-kernel
On Thu, Apr 21, 2016 at 08:11:18PM +0000, Stefan Wahren wrote:
> This patch implements suspend/resume support for mxs-lradc.
> It's possible to use the touchscreen as wakeup source.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index cfc558f..6bd387f 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -29,6 +29,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/pm.h>
> #include <linux/slab.h>
> #include <linux/stmp_device.h>
> #include <linux/sysfs.h>
> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
> + int ret = 0;
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Enable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = enable_irq_wake(lradc->irq[0]);
> + else
> + mxs_lradc_disable_ts(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + if (ret)
> + return ret;
I'd rather we had it right after we do:
ret = enable_irq_wake(lradc->irq[0]);
> +
> + mxs_lradc_hw_stop(lradc);
> +
> + clk_disable_unprepare(lradc->clk);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused mxs_lradc_resume(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
> + int ret;
> +
> + ret = clk_prepare_enable(lradc->clk);
> + if (ret)
> + return ret;
> +
> + mxs_lradc_hw_init(lradc);
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Disable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = disable_irq_wake(lradc->irq[0]);
Do we need to fail resume if this call fails?
> + else
> + mxs_lradc_enable_touch_detection(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
> +
> static struct platform_driver mxs_lradc_driver = {
> .driver = {
> .name = DRIVER_NAME,
> .of_match_table = mxs_lradc_dt_ids,
> + .pm = &mxs_lradc_pm_ops,
> },
> .probe = mxs_lradc_probe,
> .remove = mxs_lradc_remove,
> --
> 1.7.9.5
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-21 21:08 ` Dmitry Torokhov
0 siblings, 0 replies; 35+ messages in thread
From: Dmitry Torokhov @ 2016-04-21 21:08 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 21, 2016 at 08:11:18PM +0000, Stefan Wahren wrote:
> This patch implements suspend/resume support for mxs-lradc.
> It's possible to use the touchscreen as wakeup source.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> index cfc558f..6bd387f 100644
> --- a/drivers/iio/adc/mxs-lradc.c
> +++ b/drivers/iio/adc/mxs-lradc.c
> @@ -29,6 +29,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/pm.h>
> #include <linux/slab.h>
> #include <linux/stmp_device.h>
> #include <linux/sysfs.h>
> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
> + int ret = 0;
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Enable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = enable_irq_wake(lradc->irq[0]);
> + else
> + mxs_lradc_disable_ts(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + if (ret)
> + return ret;
I'd rather we had it right after we do:
ret = enable_irq_wake(lradc->irq[0]);
> +
> + mxs_lradc_hw_stop(lradc);
> +
> + clk_disable_unprepare(lradc->clk);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused mxs_lradc_resume(struct device *dev)
> +{
> + struct iio_dev *iio = dev_get_drvdata(dev);
> + struct mxs_lradc *lradc = iio_priv(iio);
> + struct input_dev *input = lradc->ts_input;
> + int ret;
> +
> + ret = clk_prepare_enable(lradc->clk);
> + if (ret)
> + return ret;
> +
> + mxs_lradc_hw_init(lradc);
> +
> + if (input) {
> + mutex_lock(&input->mutex);
> +
> + /* Disable touchscreen wakeup irq */
> + if (input->users && device_may_wakeup(dev))
> + ret = disable_irq_wake(lradc->irq[0]);
Do we need to fail resume if this call fails?
> + else
> + mxs_lradc_enable_touch_detection(lradc);
> +
> + mutex_unlock(&input->mutex);
> + }
> +
> + return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
> +
> static struct platform_driver mxs_lradc_driver = {
> .driver = {
> .name = DRIVER_NAME,
> .of_match_table = mxs_lradc_dt_ids,
> + .pm = &mxs_lradc_pm_ops,
> },
> .probe = mxs_lradc_probe,
> .remove = mxs_lradc_remove,
> --
> 1.7.9.5
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
2016-04-21 21:08 ` Dmitry Torokhov
(?)
@ 2016-04-22 13:32 ` Stefan Wahren
-1 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-22 13:32 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Marek Vasut, Lars-Peter Clausen,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Peter Meerwald-Stadler, Hartmut Knaack, Fabio Estevam,
Jonathan Cameron
Hi Dmitry,
Am 21.04.2016 um 23:08 schrieb Dmitry Torokhov:
> On Thu, Apr 21, 2016 at 08:11:18PM +0000, Stefan Wahren wrote:
>> This patch implements suspend/resume support for mxs-lradc.
>> It's possible to use the touchscreen as wakeup source.
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>> ---
>> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 61 insertions(+)
>>
>> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
>> index cfc558f..6bd387f 100644
>> --- a/drivers/iio/adc/mxs-lradc.c
>> +++ b/drivers/iio/adc/mxs-lradc.c
>> @@ -29,6 +29,7 @@
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> +#include <linux/pm.h>
>> #include <linux/slab.h>
>> #include <linux/stmp_device.h>
>> #include <linux/sysfs.h>
>> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
>> +{
>> + struct iio_dev *iio = dev_get_drvdata(dev);
>> + struct mxs_lradc *lradc = iio_priv(iio);
>> + struct input_dev *input = lradc->ts_input;
>> + int ret = 0;
>> +
>> + if (input) {
>> + mutex_lock(&input->mutex);
>> +
>> + /* Enable touchscreen wakeup irq */
>> + if (input->users && device_may_wakeup(dev))
>> + ret = enable_irq_wake(lradc->irq[0]);
>> + else
>> + mxs_lradc_disable_ts(lradc);
>> +
>> + mutex_unlock(&input->mutex);
>> + }
>> +
>> + if (ret)
>> + return ret;
> I'd rather we had it right after we do:
>
> ret = enable_irq_wake(lradc->irq[0]);
but we must unlock the mutex first before return. An option would be to
place the return inside the input branch.
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-22 13:32 ` Stefan Wahren
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-22 13:32 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Marek Vasut, Lars-Peter Clausen, linux-iio, linux-input,
linux-arm-kernel, Peter Meerwald-Stadler, Hartmut Knaack,
Fabio Estevam, Jonathan Cameron
Hi Dmitry,
Am 21.04.2016 um 23:08 schrieb Dmitry Torokhov:
> On Thu, Apr 21, 2016 at 08:11:18PM +0000, Stefan Wahren wrote:
>> This patch implements suspend/resume support for mxs-lradc.
>> It's possible to use the touchscreen as wakeup source.
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>> ---
>> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 61 insertions(+)
>>
>> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
>> index cfc558f..6bd387f 100644
>> --- a/drivers/iio/adc/mxs-lradc.c
>> +++ b/drivers/iio/adc/mxs-lradc.c
>> @@ -29,6 +29,7 @@
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> +#include <linux/pm.h>
>> #include <linux/slab.h>
>> #include <linux/stmp_device.h>
>> #include <linux/sysfs.h>
>> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
>> +{
>> + struct iio_dev *iio = dev_get_drvdata(dev);
>> + struct mxs_lradc *lradc = iio_priv(iio);
>> + struct input_dev *input = lradc->ts_input;
>> + int ret = 0;
>> +
>> + if (input) {
>> + mutex_lock(&input->mutex);
>> +
>> + /* Enable touchscreen wakeup irq */
>> + if (input->users && device_may_wakeup(dev))
>> + ret = enable_irq_wake(lradc->irq[0]);
>> + else
>> + mxs_lradc_disable_ts(lradc);
>> +
>> + mutex_unlock(&input->mutex);
>> + }
>> +
>> + if (ret)
>> + return ret;
> I'd rather we had it right after we do:
>
> ret = enable_irq_wake(lradc->irq[0]);
but we must unlock the mutex first before return. An option would be to
place the return inside the input branch.
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-22 13:32 ` Stefan Wahren
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-22 13:32 UTC (permalink / raw)
To: linux-arm-kernel
Hi Dmitry,
Am 21.04.2016 um 23:08 schrieb Dmitry Torokhov:
> On Thu, Apr 21, 2016 at 08:11:18PM +0000, Stefan Wahren wrote:
>> This patch implements suspend/resume support for mxs-lradc.
>> It's possible to use the touchscreen as wakeup source.
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>> ---
>> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 61 insertions(+)
>>
>> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
>> index cfc558f..6bd387f 100644
>> --- a/drivers/iio/adc/mxs-lradc.c
>> +++ b/drivers/iio/adc/mxs-lradc.c
>> @@ -29,6 +29,7 @@
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> +#include <linux/pm.h>
>> #include <linux/slab.h>
>> #include <linux/stmp_device.h>
>> #include <linux/sysfs.h>
>> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
>> +{
>> + struct iio_dev *iio = dev_get_drvdata(dev);
>> + struct mxs_lradc *lradc = iio_priv(iio);
>> + struct input_dev *input = lradc->ts_input;
>> + int ret = 0;
>> +
>> + if (input) {
>> + mutex_lock(&input->mutex);
>> +
>> + /* Enable touchscreen wakeup irq */
>> + if (input->users && device_may_wakeup(dev))
>> + ret = enable_irq_wake(lradc->irq[0]);
>> + else
>> + mxs_lradc_disable_ts(lradc);
>> +
>> + mutex_unlock(&input->mutex);
>> + }
>> +
>> + if (ret)
>> + return ret;
> I'd rather we had it right after we do:
>
> ret = enable_irq_wake(lradc->irq[0]);
but we must unlock the mutex first before return. An option would be to
place the return inside the input branch.
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
2016-04-21 20:45 ` Marek Vasut
(?)
@ 2016-04-22 13:57 ` Stefan Wahren
-1 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-22 13:57 UTC (permalink / raw)
To: Marek Vasut, Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: Fabio Estevam, Juergen Borleis, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Hi Marek,
Am 21.04.2016 um 22:45 schrieb Marek Vasut:
> On 04/21/2016 10:11 PM, Stefan Wahren wrote:
>> This patch implements suspend/resume support for mxs-lradc.
>> It's possible to use the touchscreen as wakeup source.
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>> ---
>> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 61 insertions(+)
>>
>> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
>> index cfc558f..6bd387f 100644
>> --- a/drivers/iio/adc/mxs-lradc.c
>> +++ b/drivers/iio/adc/mxs-lradc.c
>> @@ -29,6 +29,7 @@
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> +#include <linux/pm.h>
>> #include <linux/slab.h>
>> #include <linux/stmp_device.h>
>> #include <linux/sysfs.h>
>> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
>> +{
>> + struct iio_dev *iio = dev_get_drvdata(dev);
>> + struct mxs_lradc *lradc = iio_priv(iio);
>> + struct input_dev *input = lradc->ts_input;
> Can lradc->ts_input contain some non-valid pointer ? You should make
> sure it's either inited to NULL or valid pointer, which I'm not sure
> you do.
the whole content of lradc get initialized with zero by
devm_iio_device_alloc.
Or do you mean the error case that input registration failed and a
suspend occur (if it's possible)|?
Stefan
|
>
>> + int ret = 0;
>> +
>> + if (input) {
>> + mutex_lock(&input->mutex);
>> +
>> + /* Enable touchscreen wakeup irq */
>> + if (input->users && device_may_wakeup(dev))
>> + ret = enable_irq_wake(lradc->irq[0]);
>> + else
>> + mxs_lradc_disable_ts(lradc);
>> +
>> + mutex_unlock(&input->mutex);
>> + }
>> +
>> + if (ret)
>> + return ret;
>> +
>> + mxs_lradc_hw_stop(lradc);
>> +
>> + clk_disable_unprepare(lradc->clk);
>> +
>> + return ret;
>> +}
>> +
>> +static int __maybe_unused mxs_lradc_resume(struct device *dev)
>> +{
>> + struct iio_dev *iio = dev_get_drvdata(dev);
>> + struct mxs_lradc *lradc = iio_priv(iio);
>> + struct input_dev *input = lradc->ts_input;
>> + int ret;
>> +
>> + ret = clk_prepare_enable(lradc->clk);
>> + if (ret)
>> + return ret;
>> +
>> + mxs_lradc_hw_init(lradc);
>> +
>> + if (input) {
>> + mutex_lock(&input->mutex);
>> +
>> + /* Disable touchscreen wakeup irq */
>> + if (input->users && device_may_wakeup(dev))
>> + ret = disable_irq_wake(lradc->irq[0]);
>> + else
>> + mxs_lradc_enable_touch_detection(lradc);
>> +
>> + mutex_unlock(&input->mutex);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
>> +
>> static struct platform_driver mxs_lradc_driver = {
>> .driver = {
>> .name = DRIVER_NAME,
>> .of_match_table = mxs_lradc_dt_ids,
>> + .pm = &mxs_lradc_pm_ops,
>> },
>> .probe = mxs_lradc_probe,
>> .remove = mxs_lradc_remove,
>>
> Looks OK otherwise:
>
> Reviewed-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-22 13:57 ` Stefan Wahren
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-22 13:57 UTC (permalink / raw)
To: Marek Vasut, Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: Fabio Estevam, Juergen Borleis, linux-iio, linux-input, linux-arm-kernel
Hi Marek,
Am 21.04.2016 um 22:45 schrieb Marek Vasut:
> On 04/21/2016 10:11 PM, Stefan Wahren wrote:
>> This patch implements suspend/resume support for mxs-lradc.
>> It's possible to use the touchscreen as wakeup source.
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>> ---
>> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 61 insertions(+)
>>
>> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
>> index cfc558f..6bd387f 100644
>> --- a/drivers/iio/adc/mxs-lradc.c
>> +++ b/drivers/iio/adc/mxs-lradc.c
>> @@ -29,6 +29,7 @@
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> +#include <linux/pm.h>
>> #include <linux/slab.h>
>> #include <linux/stmp_device.h>
>> #include <linux/sysfs.h>
>> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
>> +{
>> + struct iio_dev *iio = dev_get_drvdata(dev);
>> + struct mxs_lradc *lradc = iio_priv(iio);
>> + struct input_dev *input = lradc->ts_input;
> Can lradc->ts_input contain some non-valid pointer ? You should make
> sure it's either inited to NULL or valid pointer, which I'm not sure
> you do.
the whole content of lradc get initialized with zero by
devm_iio_device_alloc.
Or do you mean the error case that input registration failed and a
suspend occur (if it's possible)|?
Stefan
|
>
>> + int ret = 0;
>> +
>> + if (input) {
>> + mutex_lock(&input->mutex);
>> +
>> + /* Enable touchscreen wakeup irq */
>> + if (input->users && device_may_wakeup(dev))
>> + ret = enable_irq_wake(lradc->irq[0]);
>> + else
>> + mxs_lradc_disable_ts(lradc);
>> +
>> + mutex_unlock(&input->mutex);
>> + }
>> +
>> + if (ret)
>> + return ret;
>> +
>> + mxs_lradc_hw_stop(lradc);
>> +
>> + clk_disable_unprepare(lradc->clk);
>> +
>> + return ret;
>> +}
>> +
>> +static int __maybe_unused mxs_lradc_resume(struct device *dev)
>> +{
>> + struct iio_dev *iio = dev_get_drvdata(dev);
>> + struct mxs_lradc *lradc = iio_priv(iio);
>> + struct input_dev *input = lradc->ts_input;
>> + int ret;
>> +
>> + ret = clk_prepare_enable(lradc->clk);
>> + if (ret)
>> + return ret;
>> +
>> + mxs_lradc_hw_init(lradc);
>> +
>> + if (input) {
>> + mutex_lock(&input->mutex);
>> +
>> + /* Disable touchscreen wakeup irq */
>> + if (input->users && device_may_wakeup(dev))
>> + ret = disable_irq_wake(lradc->irq[0]);
>> + else
>> + mxs_lradc_enable_touch_detection(lradc);
>> +
>> + mutex_unlock(&input->mutex);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
>> +
>> static struct platform_driver mxs_lradc_driver = {
>> .driver = {
>> .name = DRIVER_NAME,
>> .of_match_table = mxs_lradc_dt_ids,
>> + .pm = &mxs_lradc_pm_ops,
>> },
>> .probe = mxs_lradc_probe,
>> .remove = mxs_lradc_remove,
>>
> Looks OK otherwise:
>
> Reviewed-by: Marek Vasut <marex@denx.de>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-22 13:57 ` Stefan Wahren
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Wahren @ 2016-04-22 13:57 UTC (permalink / raw)
To: linux-arm-kernel
Hi Marek,
Am 21.04.2016 um 22:45 schrieb Marek Vasut:
> On 04/21/2016 10:11 PM, Stefan Wahren wrote:
>> This patch implements suspend/resume support for mxs-lradc.
>> It's possible to use the touchscreen as wakeup source.
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>> ---
>> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 61 insertions(+)
>>
>> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
>> index cfc558f..6bd387f 100644
>> --- a/drivers/iio/adc/mxs-lradc.c
>> +++ b/drivers/iio/adc/mxs-lradc.c
>> @@ -29,6 +29,7 @@
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> +#include <linux/pm.h>
>> #include <linux/slab.h>
>> #include <linux/stmp_device.h>
>> #include <linux/sysfs.h>
>> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
>> +{
>> + struct iio_dev *iio = dev_get_drvdata(dev);
>> + struct mxs_lradc *lradc = iio_priv(iio);
>> + struct input_dev *input = lradc->ts_input;
> Can lradc->ts_input contain some non-valid pointer ? You should make
> sure it's either inited to NULL or valid pointer, which I'm not sure
> you do.
the whole content of lradc get initialized with zero by
devm_iio_device_alloc.
Or do you mean the error case that input registration failed and a
suspend occur (if it's possible)|?
Stefan
|
>
>> + int ret = 0;
>> +
>> + if (input) {
>> + mutex_lock(&input->mutex);
>> +
>> + /* Enable touchscreen wakeup irq */
>> + if (input->users && device_may_wakeup(dev))
>> + ret = enable_irq_wake(lradc->irq[0]);
>> + else
>> + mxs_lradc_disable_ts(lradc);
>> +
>> + mutex_unlock(&input->mutex);
>> + }
>> +
>> + if (ret)
>> + return ret;
>> +
>> + mxs_lradc_hw_stop(lradc);
>> +
>> + clk_disable_unprepare(lradc->clk);
>> +
>> + return ret;
>> +}
>> +
>> +static int __maybe_unused mxs_lradc_resume(struct device *dev)
>> +{
>> + struct iio_dev *iio = dev_get_drvdata(dev);
>> + struct mxs_lradc *lradc = iio_priv(iio);
>> + struct input_dev *input = lradc->ts_input;
>> + int ret;
>> +
>> + ret = clk_prepare_enable(lradc->clk);
>> + if (ret)
>> + return ret;
>> +
>> + mxs_lradc_hw_init(lradc);
>> +
>> + if (input) {
>> + mutex_lock(&input->mutex);
>> +
>> + /* Disable touchscreen wakeup irq */
>> + if (input->users && device_may_wakeup(dev))
>> + ret = disable_irq_wake(lradc->irq[0]);
>> + else
>> + mxs_lradc_enable_touch_detection(lradc);
>> +
>> + mutex_unlock(&input->mutex);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
>> +
>> static struct platform_driver mxs_lradc_driver = {
>> .driver = {
>> .name = DRIVER_NAME,
>> .of_match_table = mxs_lradc_dt_ids,
>> + .pm = &mxs_lradc_pm_ops,
>> },
>> .probe = mxs_lradc_probe,
>> .remove = mxs_lradc_remove,
>>
> Looks OK otherwise:
>
> Reviewed-by: Marek Vasut <marex@denx.de>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
2016-04-22 13:57 ` Stefan Wahren
@ 2016-04-22 15:47 ` Marek Vasut
-1 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2016-04-22 15:47 UTC (permalink / raw)
To: Stefan Wahren, Jonathan Cameron, Dmitry Torokhov, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
Cc: Fabio Estevam, Juergen Borleis, linux-iio, linux-input, linux-arm-kernel
On 04/22/2016 03:57 PM, Stefan Wahren wrote:
> Hi Marek,
>
> Am 21.04.2016 um 22:45 schrieb Marek Vasut:
>> On 04/21/2016 10:11 PM, Stefan Wahren wrote:
>>> This patch implements suspend/resume support for mxs-lradc.
>>> It's possible to use the touchscreen as wakeup source.
>>>
>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>>> ---
>>> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 61 insertions(+)
>>>
>>> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
>>> index cfc558f..6bd387f 100644
>>> --- a/drivers/iio/adc/mxs-lradc.c
>>> +++ b/drivers/iio/adc/mxs-lradc.c
>>> @@ -29,6 +29,7 @@
>>> #include <linux/of.h>
>>> #include <linux/of_device.h>
>>> #include <linux/platform_device.h>
>>> +#include <linux/pm.h>
>>> #include <linux/slab.h>
>>> #include <linux/stmp_device.h>
>>> #include <linux/sysfs.h>
>>> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
>>> return 0;
>>> }
>>>
>>> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
>>> +{
>>> + struct iio_dev *iio = dev_get_drvdata(dev);
>>> + struct mxs_lradc *lradc = iio_priv(iio);
>>> + struct input_dev *input = lradc->ts_input;
>> Can lradc->ts_input contain some non-valid pointer ? You should make
>> sure it's either inited to NULL or valid pointer, which I'm not sure
>> you do.
>
> the whole content of lradc get initialized with zero by
> devm_iio_device_alloc.
Ah, good.
> Or do you mean the error case that input registration failed and a
> suspend occur (if it's possible)|?
No, you cannot get suspend call before you registered the device.
> Stefan
>
> |
>>
>>> + int ret = 0;
>>> +
>>> + if (input) {
>>> + mutex_lock(&input->mutex);
>>> +
>>> + /* Enable touchscreen wakeup irq */
>>> + if (input->users && device_may_wakeup(dev))
>>> + ret = enable_irq_wake(lradc->irq[0]);
>>> + else
>>> + mxs_lradc_disable_ts(lradc);
>>> +
>>> + mutex_unlock(&input->mutex);
>>> + }
>>> +
>>> + if (ret)
>>> + return ret;
>>> +
>>> + mxs_lradc_hw_stop(lradc);
>>> +
>>> + clk_disable_unprepare(lradc->clk);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int __maybe_unused mxs_lradc_resume(struct device *dev)
>>> +{
>>> + struct iio_dev *iio = dev_get_drvdata(dev);
>>> + struct mxs_lradc *lradc = iio_priv(iio);
>>> + struct input_dev *input = lradc->ts_input;
>>> + int ret;
>>> +
>>> + ret = clk_prepare_enable(lradc->clk);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + mxs_lradc_hw_init(lradc);
>>> +
>>> + if (input) {
>>> + mutex_lock(&input->mutex);
>>> +
>>> + /* Disable touchscreen wakeup irq */
>>> + if (input->users && device_may_wakeup(dev))
>>> + ret = disable_irq_wake(lradc->irq[0]);
>>> + else
>>> + mxs_lradc_enable_touch_detection(lradc);
>>> +
>>> + mutex_unlock(&input->mutex);
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
>>> +
>>> static struct platform_driver mxs_lradc_driver = {
>>> .driver = {
>>> .name = DRIVER_NAME,
>>> .of_match_table = mxs_lradc_dt_ids,
>>> + .pm = &mxs_lradc_pm_ops,
>>> },
>>> .probe = mxs_lradc_probe,
>>> .remove = mxs_lradc_remove,
>>>
>> Looks OK otherwise:
>>
>> Reviewed-by: Marek Vasut <marex@denx.de>
>>
>
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-22 15:47 ` Marek Vasut
0 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2016-04-22 15:47 UTC (permalink / raw)
To: linux-arm-kernel
On 04/22/2016 03:57 PM, Stefan Wahren wrote:
> Hi Marek,
>
> Am 21.04.2016 um 22:45 schrieb Marek Vasut:
>> On 04/21/2016 10:11 PM, Stefan Wahren wrote:
>>> This patch implements suspend/resume support for mxs-lradc.
>>> It's possible to use the touchscreen as wakeup source.
>>>
>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>>> ---
>>> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 61 insertions(+)
>>>
>>> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
>>> index cfc558f..6bd387f 100644
>>> --- a/drivers/iio/adc/mxs-lradc.c
>>> +++ b/drivers/iio/adc/mxs-lradc.c
>>> @@ -29,6 +29,7 @@
>>> #include <linux/of.h>
>>> #include <linux/of_device.h>
>>> #include <linux/platform_device.h>
>>> +#include <linux/pm.h>
>>> #include <linux/slab.h>
>>> #include <linux/stmp_device.h>
>>> #include <linux/sysfs.h>
>>> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
>>> return 0;
>>> }
>>>
>>> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
>>> +{
>>> + struct iio_dev *iio = dev_get_drvdata(dev);
>>> + struct mxs_lradc *lradc = iio_priv(iio);
>>> + struct input_dev *input = lradc->ts_input;
>> Can lradc->ts_input contain some non-valid pointer ? You should make
>> sure it's either inited to NULL or valid pointer, which I'm not sure
>> you do.
>
> the whole content of lradc get initialized with zero by
> devm_iio_device_alloc.
Ah, good.
> Or do you mean the error case that input registration failed and a
> suspend occur (if it's possible)|?
No, you cannot get suspend call before you registered the device.
> Stefan
>
> |
>>
>>> + int ret = 0;
>>> +
>>> + if (input) {
>>> + mutex_lock(&input->mutex);
>>> +
>>> + /* Enable touchscreen wakeup irq */
>>> + if (input->users && device_may_wakeup(dev))
>>> + ret = enable_irq_wake(lradc->irq[0]);
>>> + else
>>> + mxs_lradc_disable_ts(lradc);
>>> +
>>> + mutex_unlock(&input->mutex);
>>> + }
>>> +
>>> + if (ret)
>>> + return ret;
>>> +
>>> + mxs_lradc_hw_stop(lradc);
>>> +
>>> + clk_disable_unprepare(lradc->clk);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int __maybe_unused mxs_lradc_resume(struct device *dev)
>>> +{
>>> + struct iio_dev *iio = dev_get_drvdata(dev);
>>> + struct mxs_lradc *lradc = iio_priv(iio);
>>> + struct input_dev *input = lradc->ts_input;
>>> + int ret;
>>> +
>>> + ret = clk_prepare_enable(lradc->clk);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + mxs_lradc_hw_init(lradc);
>>> +
>>> + if (input) {
>>> + mutex_lock(&input->mutex);
>>> +
>>> + /* Disable touchscreen wakeup irq */
>>> + if (input->users && device_may_wakeup(dev))
>>> + ret = disable_irq_wake(lradc->irq[0]);
>>> + else
>>> + mxs_lradc_enable_touch_detection(lradc);
>>> +
>>> + mutex_unlock(&input->mutex);
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static SIMPLE_DEV_PM_OPS(mxs_lradc_pm_ops, mxs_lradc_suspend, mxs_lradc_resume);
>>> +
>>> static struct platform_driver mxs_lradc_driver = {
>>> .driver = {
>>> .name = DRIVER_NAME,
>>> .of_match_table = mxs_lradc_dt_ids,
>>> + .pm = &mxs_lradc_pm_ops,
>>> },
>>> .probe = mxs_lradc_probe,
>>> .remove = mxs_lradc_remove,
>>>
>> Looks OK otherwise:
>>
>> Reviewed-by: Marek Vasut <marex@denx.de>
>>
>
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
2016-04-22 13:32 ` Stefan Wahren
(?)
@ 2016-04-25 21:14 ` Dmitry Torokhov
-1 siblings, 0 replies; 35+ messages in thread
From: Dmitry Torokhov @ 2016-04-25 21:14 UTC (permalink / raw)
To: Stefan Wahren
Cc: Marek Vasut, Lars-Peter Clausen,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Peter Meerwald-Stadler, Hartmut Knaack, Fabio Estevam,
Jonathan Cameron
On Fri, Apr 22, 2016 at 03:32:44PM +0200, Stefan Wahren wrote:
> Hi Dmitry,
>
> Am 21.04.2016 um 23:08 schrieb Dmitry Torokhov:
> > On Thu, Apr 21, 2016 at 08:11:18PM +0000, Stefan Wahren wrote:
> >> This patch implements suspend/resume support for mxs-lradc.
> >> It's possible to use the touchscreen as wakeup source.
> >>
> >> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> >> ---
> >> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 61 insertions(+)
> >>
> >> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> >> index cfc558f..6bd387f 100644
> >> --- a/drivers/iio/adc/mxs-lradc.c
> >> +++ b/drivers/iio/adc/mxs-lradc.c
> >> @@ -29,6 +29,7 @@
> >> #include <linux/of.h>
> >> #include <linux/of_device.h>
> >> #include <linux/platform_device.h>
> >> +#include <linux/pm.h>
> >> #include <linux/slab.h>
> >> #include <linux/stmp_device.h>
> >> #include <linux/sysfs.h>
> >> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
> >> return 0;
> >> }
> >>
> >> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
> >> +{
> >> + struct iio_dev *iio = dev_get_drvdata(dev);
> >> + struct mxs_lradc *lradc = iio_priv(iio);
> >> + struct input_dev *input = lradc->ts_input;
> >> + int ret = 0;
> >> +
> >> + if (input) {
> >> + mutex_lock(&input->mutex);
> >> +
> >> + /* Enable touchscreen wakeup irq */
> >> + if (input->users && device_may_wakeup(dev))
> >> + ret = enable_irq_wake(lradc->irq[0]);
> >> + else
> >> + mxs_lradc_disable_ts(lradc);
> >> +
> >> + mutex_unlock(&input->mutex);
> >> + }
> >> +
> >> + if (ret)
> >> + return ret;
> > I'd rather we had it right after we do:
> >
> > ret = enable_irq_wake(lradc->irq[0]);
>
> but we must unlock the mutex first before return. An option would be to
> place the return inside the input branch.
Ah, right, ignore me then.
--
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-25 21:14 ` Dmitry Torokhov
0 siblings, 0 replies; 35+ messages in thread
From: Dmitry Torokhov @ 2016-04-25 21:14 UTC (permalink / raw)
To: Stefan Wahren
Cc: Marek Vasut, Lars-Peter Clausen, linux-iio, linux-input,
linux-arm-kernel, Peter Meerwald-Stadler, Hartmut Knaack,
Fabio Estevam, Jonathan Cameron
On Fri, Apr 22, 2016 at 03:32:44PM +0200, Stefan Wahren wrote:
> Hi Dmitry,
>
> Am 21.04.2016 um 23:08 schrieb Dmitry Torokhov:
> > On Thu, Apr 21, 2016 at 08:11:18PM +0000, Stefan Wahren wrote:
> >> This patch implements suspend/resume support for mxs-lradc.
> >> It's possible to use the touchscreen as wakeup source.
> >>
> >> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> >> ---
> >> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 61 insertions(+)
> >>
> >> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> >> index cfc558f..6bd387f 100644
> >> --- a/drivers/iio/adc/mxs-lradc.c
> >> +++ b/drivers/iio/adc/mxs-lradc.c
> >> @@ -29,6 +29,7 @@
> >> #include <linux/of.h>
> >> #include <linux/of_device.h>
> >> #include <linux/platform_device.h>
> >> +#include <linux/pm.h>
> >> #include <linux/slab.h>
> >> #include <linux/stmp_device.h>
> >> #include <linux/sysfs.h>
> >> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
> >> return 0;
> >> }
> >>
> >> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
> >> +{
> >> + struct iio_dev *iio = dev_get_drvdata(dev);
> >> + struct mxs_lradc *lradc = iio_priv(iio);
> >> + struct input_dev *input = lradc->ts_input;
> >> + int ret = 0;
> >> +
> >> + if (input) {
> >> + mutex_lock(&input->mutex);
> >> +
> >> + /* Enable touchscreen wakeup irq */
> >> + if (input->users && device_may_wakeup(dev))
> >> + ret = enable_irq_wake(lradc->irq[0]);
> >> + else
> >> + mxs_lradc_disable_ts(lradc);
> >> +
> >> + mutex_unlock(&input->mutex);
> >> + }
> >> +
> >> + if (ret)
> >> + return ret;
> > I'd rather we had it right after we do:
> >
> > ret = enable_irq_wake(lradc->irq[0]);
>
> but we must unlock the mutex first before return. An option would be to
> place the return inside the input branch.
Ah, right, ignore me then.
--
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support
@ 2016-04-25 21:14 ` Dmitry Torokhov
0 siblings, 0 replies; 35+ messages in thread
From: Dmitry Torokhov @ 2016-04-25 21:14 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 22, 2016 at 03:32:44PM +0200, Stefan Wahren wrote:
> Hi Dmitry,
>
> Am 21.04.2016 um 23:08 schrieb Dmitry Torokhov:
> > On Thu, Apr 21, 2016 at 08:11:18PM +0000, Stefan Wahren wrote:
> >> This patch implements suspend/resume support for mxs-lradc.
> >> It's possible to use the touchscreen as wakeup source.
> >>
> >> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> >> ---
> >> drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 61 insertions(+)
> >>
> >> diff --git a/drivers/iio/adc/mxs-lradc.c b/drivers/iio/adc/mxs-lradc.c
> >> index cfc558f..6bd387f 100644
> >> --- a/drivers/iio/adc/mxs-lradc.c
> >> +++ b/drivers/iio/adc/mxs-lradc.c
> >> @@ -29,6 +29,7 @@
> >> #include <linux/of.h>
> >> #include <linux/of_device.h>
> >> #include <linux/platform_device.h>
> >> +#include <linux/pm.h>
> >> #include <linux/slab.h>
> >> #include <linux/stmp_device.h>
> >> #include <linux/sysfs.h>
> >> @@ -1745,10 +1746,70 @@ static int mxs_lradc_remove(struct platform_device *pdev)
> >> return 0;
> >> }
> >>
> >> +static int __maybe_unused mxs_lradc_suspend(struct device *dev)
> >> +{
> >> + struct iio_dev *iio = dev_get_drvdata(dev);
> >> + struct mxs_lradc *lradc = iio_priv(iio);
> >> + struct input_dev *input = lradc->ts_input;
> >> + int ret = 0;
> >> +
> >> + if (input) {
> >> + mutex_lock(&input->mutex);
> >> +
> >> + /* Enable touchscreen wakeup irq */
> >> + if (input->users && device_may_wakeup(dev))
> >> + ret = enable_irq_wake(lradc->irq[0]);
> >> + else
> >> + mxs_lradc_disable_ts(lradc);
> >> +
> >> + mutex_unlock(&input->mutex);
> >> + }
> >> +
> >> + if (ret)
> >> + return ret;
> > I'd rather we had it right after we do:
> >
> > ret = enable_irq_wake(lradc->irq[0]);
>
> but we must unlock the mutex first before return. An option would be to
> place the return inside the input branch.
Ah, right, ignore me then.
--
Dmitry
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2016-04-25 21:14 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-21 20:11 [PATCH RFT V2 0/3] iio: mxs-lradc: implement PM ops Stefan Wahren
2016-04-21 20:11 ` Stefan Wahren
2016-04-21 20:11 ` Stefan Wahren
2016-04-21 20:11 ` [PATCH RFT V2 1/3] iio: mxs-lradc: simplify TS registration Stefan Wahren
2016-04-21 20:11 ` Stefan Wahren
[not found] ` <1461269478-449-2-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
2016-04-21 20:39 ` Marek Vasut
2016-04-21 20:39 ` Marek Vasut
2016-04-21 20:39 ` Marek Vasut
2016-04-21 21:06 ` Dmitry Torokhov
2016-04-21 21:06 ` Dmitry Torokhov
[not found] ` <1461269478-449-1-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
2016-04-21 20:11 ` [PATCH RFT V2 2/3] iio: mxs-lradc: disable only masked channels in mxs_lradc_hw_stop Stefan Wahren
2016-04-21 20:11 ` Stefan Wahren
2016-04-21 20:11 ` Stefan Wahren
[not found] ` <1461269478-449-3-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
2016-04-21 20:44 ` Marek Vasut
2016-04-21 20:44 ` Marek Vasut
2016-04-21 20:44 ` Marek Vasut
2016-04-21 20:11 ` [PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support Stefan Wahren
2016-04-21 20:11 ` Stefan Wahren
[not found] ` <1461269478-449-4-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
2016-04-21 20:45 ` Marek Vasut
2016-04-21 20:45 ` Marek Vasut
2016-04-21 20:45 ` Marek Vasut
[not found] ` <57193C01.20709-ynQEQJNshbs@public.gmane.org>
2016-04-22 13:57 ` Stefan Wahren
2016-04-22 13:57 ` Stefan Wahren
2016-04-22 13:57 ` Stefan Wahren
2016-04-22 15:47 ` Marek Vasut
2016-04-22 15:47 ` Marek Vasut
2016-04-21 21:08 ` Dmitry Torokhov
2016-04-21 21:08 ` Dmitry Torokhov
2016-04-21 21:08 ` Dmitry Torokhov
2016-04-22 13:32 ` Stefan Wahren
2016-04-22 13:32 ` Stefan Wahren
2016-04-22 13:32 ` Stefan Wahren
[not found] ` <571A27FC.4030301-eS4NqCHxEME@public.gmane.org>
2016-04-25 21:14 ` Dmitry Torokhov
2016-04-25 21:14 ` Dmitry Torokhov
2016-04-25 21:14 ` Dmitry Torokhov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.