iio: mxs-lradc: fix leak and implement PM ops
Subsystems (3)
cover iio: mxs-lradc: fix leak and implement PM ops
This patch series fix a memory leak and implements suspend/resume support for mxs-lradc.
Patch 1: fix the memory leak
Patch 2-4: refactor mxs-lradc in order to prepare PM implementation Patch 5: 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.
Stefan Wahren (5):
iio: mxs-lradc: fix memory leak
iio: mxs-lradc: move TS config into suitable function
iio: mxs-lradc: move STMP reset out of ADC init
iio: mxs-lradc: disable only mapped channels in mxs_lradc_hw_stop
iio: mxs-lradc: implement suspend/resume support
drivers/iio/adc/mxs-lradc.c | 116 ++++++++++++++++++++++++++++++------------- 1 file changed, 82 insertions(+), 34 deletions(-)
Signature · 1 line
1.7.9.5
PATCH 1/5 iio: mxs-lradc: fix memory leak
After successful touchscreen registration the input device was never freed. So fix this issue by using devm_input_allocate_device().
--- drivers/iio/adc/mxs-lradc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
drivers/iio/adc/mxs-lradc.c +2 -6
@@ -1109,12 +1109,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 ;
@@ -1134,11 +1133,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)
Signature · 1 line
1.7.9.5
PATCH 1/5 iio: mxs-lradc: fix memory leak
Quoted 30 lines · Stefan Wahren
After successful touchscreen registration the input device was never freed. So fix this issue by using
devm_input_allocate_device().Signed-off-by Stefan Wahren <[email protected]>--- drivers/iio/adc/mxs-lradc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)drivers/iio/adc/mxs-lradc.c +1
-2@@ -1109,12 +1109,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 ;@@ -1134,11 +1133,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc) lradc->ts_input = input;
drivers/iio/adc/mxs-lradc.c static int mxs_lradc_ts_register(struct mxs_lradc *lradc) · line 1140
Quoted 10 lines
drivers/iio/adc/mxs-lradc.c +1
-4input_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)
Nice find.
Looks like at91_adc.c and exynos_adc.c suffer from the exact same issue. The leak looks a bit more severe on exynos even, exynos_adc_ts_init() could use a proper fail path. Do you want to send patches or shall I ?
Signature · 2 lines
Best regards, Marek Vasut
Jonathan Cameron
maintainerPATCH 1/5 iio: mxs-lradc: fix memory leak
Quoted 39 lines · Marek Vasut
Quoted 38 lines · Stefan Wahren
After successful touchscreen registration the input device was never freed. So fix this issue by using
devm_input_allocate_device().Signed-off-by Stefan Wahren <[email protected]>--- drivers/iio/adc/mxs-lradc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)drivers/iio/adc/mxs-lradc.c +2
-6@@ -1109,12 +1109,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 ;@@ -1134,11 +1133,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); }
Quoted 9 lines
drivers/iio/adc/mxs-lradc.c static int mxs_lradc_ts_register(struct mxs_lradc *lradc) · line 1140
Quoted 2 lines
drivers/iio/adc/mxs-lradc.c +0
-0static void mxs_lradc_ts_unregister (struct mxs_lradc *lradc)Nice find.
Looks like
at91_adc.candexynos_adc.csuffer from the exact same issue. The leak looks a bit more severe on exynos even,exynos_adc_ts_init()could use a proper fail path. Do you want to send patches or shall I ?
As this has been there a long time I'm not going to rush it in as a fix. I suspect no platform is probing and removing it enough to get a significant amount of memory leaked.
Good to clear it up though!
Applied to the togreg branch of iio.git, initially pushed out as testing for the autobuilders to play with it.
Thanks,
Jonathan
PATCH 1/5 iio: mxs-lradc: fix memory leak
Quoted 45 lines · Jonathan Cameron
Quoted 44 lines · Marek Vasut
Quoted 30 lines · Stefan Wahren
After successful touchscreen registration the input device was never freed. So fix this issue by using
devm_input_allocate_device().Signed-off-by Stefan Wahren <[email protected]>--- drivers/iio/adc/mxs-lradc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)drivers/iio/adc/mxs-lradc.c +1
-2@@ -1109,12 +1109,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 ;@@ -1134,11 +1133,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc) lradc->ts_input = input;drivers/iio/adc/mxs-lradc.c static int mxs_lradc_ts_register(struct mxs_lradc *lradc) · line 1140
Quoted 10 lines
drivers/iio/adc/mxs-lradc.c +1
-4input_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)Nice find.
Quoted 5 lines
Quoted 4 lines
Looks like
at91_adc.candexynos_adc.csuffer from the exact same issue. The leak looks a bit more severe on exynos even,exynos_adc_ts_init()could use a proper fail path. Do you want to send patches or shall I ?As this has been there a long time I'm not going to rush it in as a fix.
I did take a proper look today and it seems they do the right thing afterall. I checked them with kmemleak too to be sure.
Quoted 12 lines
I suspect no platform is probing and removing it enough to get a significant amount of memory leaked.
Good to clear it up though!
Applied to the togreg branch of iio.git, initially pushed out as testing for the autobuilders to play with it.
Thanks,
Jonathan
Signature · 2 lines
Best regards, Marek Vasut
PATCH 1/5 iio: mxs-lradc: fix memory leak
Hi Marek,
Am 18.04.2016 um 19:16 schrieb Marek Vasut:
Quoted 45 lines · Jonathan Cameron
Quoted 44 lines · Marek Vasut
Quoted 43 lines · Stefan Wahren
Quoted 30 lines
After successful touchscreen registration the input device was never freed. So fix this issue by using
devm_input_allocate_device().Signed-off-by Stefan Wahren <[email protected]>--- drivers/iio/adc/mxs-lradc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)drivers/iio/adc/mxs-lradc.c +1
-2@@ -1109,12 +1109,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 ;@@ -1134,11 +1133,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc) lradc->ts_input = input;drivers/iio/adc/mxs-lradc.c static int mxs_lradc_ts_register(struct mxs_lradc *lradc) · line 1140
Quoted 10 lines
drivers/iio/adc/mxs-lradc.c +1
-4input_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)Nice find.
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
Looks like
at91_adc.candexynos_adc.csuffer from the exact same issue. The leak looks a bit more severe on exynos even,exynos_adc_ts_init()could use a proper fail path. Do you want to send patches or shall I ?As this has been there a long time I'm not going to rush it in as a fix.
I did take a proper look today and it seems they do the right thing afterall. I checked them with kmemleak too to be sure.
thanks, input_unregister_device already free the memory.
Sorry for the mess :-(
I think it would be the best to remove / revert this patch.
Stefan
Quoted 14 lines
Quoted 13 lines
I suspect no platform is probing and removing it enough to get a significant amount of memory leaked.
Good to clear it up though!
Applied to the togreg branch of iio.git, initially pushed out as testing for the autobuilders to play with it.
Thanks,
Jonathan
PATCH 1/5 iio: mxs-lradc: fix memory leak
Quoted 53 lines · Stefan Wahren
Hi Marek,
Am 18.04.2016 um 19:16 schrieb Marek Vasut:
Quoted 50 lines · Jonathan Cameron
Quoted 49 lines · Marek Vasut
Quoted 39 lines · Stefan Wahren
Quoted 38 lines
After successful touchscreen registration the input device was never freed. So fix this issue by using
devm_input_allocate_device().Signed-off-by Stefan Wahren <[email protected]>--- drivers/iio/adc/mxs-lradc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)drivers/iio/adc/mxs-lradc.c +2
-6@@ -1109,12 +1109,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 ;@@ -1134,11 +1133,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); }Quoted 8 lines
drivers/iio/adc/mxs-lradc.c static int mxs_lradc_ts_register(struct mxs_lradc *lradc) · line 1140
Quoted 2 lines
drivers/iio/adc/mxs-lradc.c +0
-0static void mxs_lradc_ts_unregister (struct mxs_lradc *lradc)Nice find.
Looks like
at91_adc.candexynos_adc.csuffer from the exact same issue. The leak looks a bit more severe on exynos even,exynos_adc_ts_init()could use a proper fail path. Do you want to send patches or shall I ?As this has been there a long time I'm not going to rush it in as a fix.
Quoted 8 lines
Quoted 2 lines
I did take a proper look today and it seems they do the right thing afterall. I checked them with kmemleak too to be sure.
thanks,
input_unregister_devicealready free the memory.Sorry for the mess :-(
I think it would be the best to remove / revert this patch.
This one? Why exactly? Please elaborate some more, so it's possible to understand the reasoning :)
Signature · 2 lines
Best regards, Marek Vasut
PATCH 1/5 iio: mxs-lradc: fix memory leak
Hi Marek,
Am 19.04.2016 um 12:32 schrieb Marek Vasut:
Quoted 56 lines · Stefan Wahren
Quoted 55 lines
Hi Marek,
Am 18.04.2016 um 19:16 schrieb Marek Vasut:
Quoted 52 lines · Jonathan Cameron
Quoted 44 lines · Marek Vasut
Quoted 43 lines · Stefan Wahren
Quoted 30 lines
After successful touchscreen registration the input device was never freed. So fix this issue by using
devm_input_allocate_device().Signed-off-by Stefan Wahren <[email protected]>--- drivers/iio/adc/mxs-lradc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)drivers/iio/adc/mxs-lradc.c +1
-2@@ -1109,12 +1109,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 ;@@ -1134,11 +1133,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc) lradc->ts_input = input;drivers/iio/adc/mxs-lradc.c static int mxs_lradc_ts_register(struct mxs_lradc *lradc) · line 1140
Quoted 10 lines
drivers/iio/adc/mxs-lradc.c +1
-4input_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)Nice find.
Quoted 5 lines
Quoted 4 lines
Looks like
at91_adc.candexynos_adc.csuffer from the exact same issue. The leak looks a bit more severe on exynos even,exynos_adc_ts_init()could use a proper fail path. Do you want to send patches or shall I ?As this has been there a long time I'm not going to rush it in as a fix.
I did take a proper look today and it seems they do the right thing afterall. I checked them with kmemleak too to be sure.
Quoted 9 lines
Quoted 5 lines
thanks,
input_unregister_devicealready free the memory.Sorry for the mess :-(
I think it would be the best to remove / revert this patch.
This one? Why exactly? Please elaborate some more, so it's possible to understand the reasoning :)
as you already pointed out this is not a memory leak, because input_unregister_device free the input device. So the commit message is incorrect.
This commit also simplifies the code but in this case we should also remove input_unregister_device.
Regards
Stefan
Jonathan Cameron
maintainerPATCH 1/5 iio: mxs-lradc: fix memory leak
Quoted 64 lines · Stefan Wahren
Hi Marek,
Am 19.04.2016 um 12:32 schrieb Marek Vasut:
Quoted 61 lines · Stefan Wahren
Quoted 60 lines
Hi Marek,
Am 18.04.2016 um 19:16 schrieb Marek Vasut:
Quoted 45 lines · Jonathan Cameron
Quoted 44 lines · Marek Vasut
Quoted 43 lines · Stefan Wahren
Quoted 30 lines
After successful touchscreen registration the input device was never freed. So fix this issue by using
devm_input_allocate_device().Signed-off-by Stefan Wahren <[email protected]>--- drivers/iio/adc/mxs-lradc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)drivers/iio/adc/mxs-lradc.c +1
-2@@ -1109,12 +1109,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 ;@@ -1134,11 +1133,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc) lradc->ts_input = input;drivers/iio/adc/mxs-lradc.c static int mxs_lradc_ts_register(struct mxs_lradc *lradc) · line 1140
Quoted 10 lines
drivers/iio/adc/mxs-lradc.c +1
-4input_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)Nice find.
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
Looks like
at91_adc.candexynos_adc.csuffer from the exact same issue. The leak looks a bit more severe on exynos even,exynos_adc_ts_init()could use a proper fail path. Do you want to send patches or shall I ?As this has been there a long time I'm not going to rush it in as a fix.
I did take a proper look today and it seems they do the right thing afterall. I checked them with kmemleak too to be sure.
thanks,
input_unregister_devicealready free the memory.Sorry for the mess :-(
I think it would be the best to remove / revert this patch.
Removed it. Had never pushed this out in a non rebasable branch so easiest just to drop it.
Jonathan
Quoted 19 lines
Quoted 4 lines
This one? Why exactly? Please elaborate some more, so it's possible to understand the reasoning :)
as you already pointed out this is not a memory leak, because
input_unregister_devicefree the input device. So the commit message is incorrect.This commit also simplifies the code but in this case we should also remove
input_unregister_device.Regards
Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/5 iio: mxs-lradc: fix memory leak
In reply to Stefan WahrenQuoted 64 lines · Stefan Wahren
Hi Marek,
Am 19.04.2016 um 12:32 schrieb Marek Vasut:
Quoted 61 lines · Stefan Wahren
Quoted 60 lines
Hi Marek,
Am 18.04.2016 um 19:16 schrieb Marek Vasut:
Quoted 45 lines · Jonathan Cameron
Quoted 44 lines · Marek Vasut
Quoted 43 lines · Stefan Wahren
Quoted 30 lines
After successful touchscreen registration the input device was never freed. So fix this issue by using
devm_input_allocate_device().Signed-off-by Stefan Wahren <[email protected]>--- drivers/iio/adc/mxs-lradc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)drivers/iio/adc/mxs-lradc.c +1
-2@@ -1109,12 +1109,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 ;@@ -1134,11 +1133,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc) lradc->ts_input = input;drivers/iio/adc/mxs-lradc.c static int mxs_lradc_ts_register(struct mxs_lradc *lradc) · line 1140
Quoted 10 lines
drivers/iio/adc/mxs-lradc.c +1
-4input_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)Nice find.
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
Looks like
at91_adc.candexynos_adc.csuffer from the exact same issue. The leak looks a bit more severe on exynos even,exynos_adc_ts_init()could use a proper fail path. Do you want to send patches or shall I ?As this has been there a long time I'm not going to rush it in as a fix.
I did take a proper look today and it seems they do the right thing afterall. I checked them with kmemleak too to be sure.
thanks,
input_unregister_devicealready free the memory.Sorry for the mess :-(
I think it would be the best to remove / revert this patch.
Quoted 8 lines
Quoted 4 lines
This one? Why exactly? Please elaborate some more, so it's possible to understand the reasoning :)
as you already pointed out this is not a memory leak, because
input_unregister_devicefree the input device. So the commit message is incorrect.
I see, yes, it's not a memleak.
Quoted 2 lines
This commit also simplifies the code but in this case we should also remove
input_unregister_device.
This matches the explanation in drivers/input/input.c above input_register_device() , agreed.
Quoted 3 lines
Regards
Stefan
Signature · 2 lines
Best regards, Marek Vasut
PATCH 1/5 iio: mxs-lradc: fix memory leak
In reply to Marek VasutHi Marek,
Am 14.04.2016 um 22:01 schrieb Marek Vasut:
Quoted 39 lines · Stefan Wahren
Quoted 38 lines
After successful touchscreen registration the input device was never freed. So fix this issue by using
devm_input_allocate_device().Signed-off-by Stefan Wahren <[email protected]>--- drivers/iio/adc/mxs-lradc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)drivers/iio/adc/mxs-lradc.c +2
-6@@ -1109,12 +1109,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 ;@@ -1134,11 +1133,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); }
Quoted 8 lines
drivers/iio/adc/mxs-lradc.c static int mxs_lradc_ts_register(struct mxs_lradc *lradc) · line 1140
Quoted 2 lines
drivers/iio/adc/mxs-lradc.c +0
-0static void mxs_lradc_ts_unregister (struct mxs_lradc *lradc)Nice find.
Looks like
at91_adc.candexynos_adc.csuffer from the exact same issue. The leak looks a bit more severe on exynos even,exynos_adc_ts_init()could use a proper fail path. Do you want to send patches or shall I ?
since you found them it would be better if you handle them.
Regards
Stefan
PATCH 1/5 iio: mxs-lradc: fix memory leak
Quoted 46 lines · Stefan Wahren
Hi Marek,
Am 14.04.2016 um 22:01 schrieb Marek Vasut:
Quoted 43 lines · Stefan Wahren
Quoted 30 lines
After successful touchscreen registration the input device was never freed. So fix this issue by using
devm_input_allocate_device().Signed-off-by Stefan Wahren <[email protected]>--- drivers/iio/adc/mxs-lradc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)drivers/iio/adc/mxs-lradc.c +1
-2@@ -1109,12 +1109,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 ;@@ -1134,11 +1133,8 @@ static int mxs_lradc_ts_register(struct mxs_lradc *lradc) lradc->ts_input = input;drivers/iio/adc/mxs-lradc.c static int mxs_lradc_ts_register(struct mxs_lradc *lradc) · line 1140
Quoted 10 lines
drivers/iio/adc/mxs-lradc.c +1
-4input_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)Nice find.
Quoted 6 lines
Quoted 4 lines
Looks like
at91_adc.candexynos_adc.csuffer from the exact same issue. The leak looks a bit more severe on exynos even,exynos_adc_ts_init()could use a proper fail path. Do you want to send patches or shall I ?since you found them it would be better if you handle them.
Roger. Since Atmel was so kind to provide sama5d2 and I also have sama5d4 board here, I will check the Atmel one at least.
Signature · 2 lines
Best regards, Marek Vasut
PATCH 2/5 iio: mxs-lradc: move TS config into suitable function
In reply to Stefan WahrenThis patch moves the touchscreen type configuration into a more suitable function. Btw this simplifies PM ops later.
--- drivers/iio/adc/mxs-lradc.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
drivers/iio/adc/mxs-lradc.c +11 -10
@@ -686,6 +686,17 @@ static void mxs_lradc_prepare_pressure(struct mxs_lradc *lradc)static void mxs_lradc_enable_touch_detection (struct mxs_lradc *lradc) { +/* Configure the touchscreen type */ +if (lradc->soc ==IMX28_LRADC ) { +mxs_lradc_reg_clear (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , +LRADC_CTRL0 ); + +if (lradc->use_touchscreen ==MXS_LRADC_TOUCHSCREEN_5WIRE ) +mxs_lradc_reg_set (lradc, +LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , +LRADC_CTRL0 ); + } +mxs_lradc_setup_touch_detection (lradc); lradc->cur_plate =LRADC_TOUCH ;
@@ -1491,16 +1502,6 @@ static int mxs_lradc_hw_init(struct mxs_lradc *lradc)mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (2 ));mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (3 )); -/* Configure the touchscreen type */ -if (lradc->soc ==IMX28_LRADC ) { -mxs_lradc_reg_clear (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , -LRADC_CTRL0 ); - -if (lradc->use_touchscreen ==MXS_LRADC_TOUCHSCREEN_5WIRE ) -mxs_lradc_reg_set (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , -LRADC_CTRL0 ); - } -/* Start internal temperature sensing. */ mxs_lradc_reg_wrt (lradc,0 ,LRADC_CTRL2 );
Signature · 1 line
1.7.9.5
PATCH 2/5 iio: mxs-lradc: move TS config into suitable function
Quoted 4 lines · Stefan Wahren
This patch moves the touchscreen type configuration into a more suitable function. Btw this simplifies PM ops later.
Signed-off-by Stefan Wahren <[email protected]>
Looks OK
Quoted 44 lines
--- drivers/iio/adc/mxs-lradc.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)drivers/iio/adc/mxs-lradc.c +11
-10@@ -686,6 +686,17 @@ static void mxs_lradc_prepare_pressure(struct mxs_lradc *lradc)static void mxs_lradc_enable_touch_detection (struct mxs_lradc *lradc) { +/* Configure the touchscreen type */ +if (lradc->soc ==IMX28_LRADC ) { +mxs_lradc_reg_clear (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , +LRADC_CTRL0 ); + +if (lradc->use_touchscreen ==MXS_LRADC_TOUCHSCREEN_5WIRE ) +mxs_lradc_reg_set (lradc, +LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , +LRADC_CTRL0 ); + } +mxs_lradc_setup_touch_detection (lradc); lradc->cur_plate =LRADC_TOUCH ;@@ -1491,16 +1502,6 @@ static int mxs_lradc_hw_init(struct mxs_lradc *lradc)mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (2 ));mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (3 )); -/* Configure the touchscreen type */ -if (lradc->soc ==IMX28_LRADC ) { -mxs_lradc_reg_clear (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , -LRADC_CTRL0 ); - -if (lradc->use_touchscreen ==MXS_LRADC_TOUCHSCREEN_5WIRE ) -mxs_lradc_reg_set (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , -LRADC_CTRL0 ); - } -/* Start internal temperature sensing. */ mxs_lradc_reg_wrt (lradc,0 ,LRADC_CTRL2 );
Signature · 2 lines
Best regards, Marek Vasut
PATCH 2/5 iio: mxs-lradc: move TS config into suitable function
Quoted 9 lines · Marek Vasut
Quoted 4 lines · Stefan Wahren
This patch moves the touchscreen type configuration into a more suitable function. Btw this simplifies PM ops later.
Signed-off-by Stefan Wahren <[email protected]>Looks OK
Reviewed-by Marek Vasut <[email protected]>
Please do CC Dmitry and linux-input on any patches 'touching' the touchscreen parts of the driver. I'm happy this one is simple enough to apply anyway, but best to make sure everyone is in the loop.
Something a little odd happened when applying the patch. Automerging failed so I did it by hand. Not sure what had changed!
Anyhow, please sanity check I haven't messed it up.
Applied to the togreg branch of iio.git - initially pushed out as testing for the autobuilders to play with it.
Thanks,
Jonathan
Quoted 47 lines
Quoted 45 lines
--- drivers/iio/adc/mxs-lradc.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)drivers/iio/adc/mxs-lradc.c +11
-10@@ -686,6 +686,17 @@ static void mxs_lradc_prepare_pressure(struct mxs_lradc *lradc)static void mxs_lradc_enable_touch_detection (struct mxs_lradc *lradc) { +/* Configure the touchscreen type */ +if (lradc->soc ==IMX28_LRADC ) { +mxs_lradc_reg_clear (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , +LRADC_CTRL0 ); + +if (lradc->use_touchscreen ==MXS_LRADC_TOUCHSCREEN_5WIRE ) +mxs_lradc_reg_set (lradc, +LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , +LRADC_CTRL0 ); + } +mxs_lradc_setup_touch_detection (lradc); lradc->cur_plate =LRADC_TOUCH ;@@ -1491,16 +1502,6 @@ static int mxs_lradc_hw_init(struct mxs_lradc *lradc)mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (2 ));mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (3 )); -/* Configure the touchscreen type */ -if (lradc->soc ==IMX28_LRADC ) { -mxs_lradc_reg_clear (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , -LRADC_CTRL0 ); - -if (lradc->use_touchscreen ==MXS_LRADC_TOUCHSCREEN_5WIRE ) -mxs_lradc_reg_set (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , -LRADC_CTRL0 ); - } -/* Start internal temperature sensing. */ mxs_lradc_reg_wrt (lradc,0 ,LRADC_CTRL2 );
PATCH 2/5 iio: mxs-lradc: move TS config into suitable function
Quoted 7 lines · Jonathan Cameron
Quoted 6 lines · Marek Vasut
Quoted 4 lines · Stefan Wahren
This patch moves the touchscreen type configuration into a more suitable function. Btw this simplifies PM ops later.
Signed-off-by Stefan Wahren <[email protected]>
Quoted 7 lines
Quoted 3 lines
Looks OK
Reviewed-by Marek Vasut <[email protected]>Please do CC Dmitry and linux-input on any patches 'touching' the touchscreen parts of the driver. I'm happy this one is simple enough to apply anyway, but best to make sure everyone is in the loop.
FWIW:
Quoted 61 lines
Something a little odd happened when applying the patch. Automerging failed so I did it by hand. Not sure what had changed!
Anyhow, please sanity check I haven't messed it up.
Applied to the togreg branch of iio.git - initially pushed out as testing for the autobuilders to play with it.
Thanks,
Jonathan
Quoted 47 lines
Quoted 45 lines
--- drivers/iio/adc/mxs-lradc.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)drivers/iio/adc/mxs-lradc.c +11
-10@@ -686,6 +686,17 @@ static void mxs_lradc_prepare_pressure(struct mxs_lradc *lradc)static void mxs_lradc_enable_touch_detection (struct mxs_lradc *lradc) { +/* Configure the touchscreen type */ +if (lradc->soc ==IMX28_LRADC ) { +mxs_lradc_reg_clear (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , +LRADC_CTRL0 ); + +if (lradc->use_touchscreen ==MXS_LRADC_TOUCHSCREEN_5WIRE ) +mxs_lradc_reg_set (lradc, +LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , +LRADC_CTRL0 ); + } +mxs_lradc_setup_touch_detection (lradc); lradc->cur_plate =LRADC_TOUCH ;@@ -1491,16 +1502,6 @@ static int mxs_lradc_hw_init(struct mxs_lradc *lradc)mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (2 ));mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (3 )); -/* Configure the touchscreen type */ -if (lradc->soc ==IMX28_LRADC ) { -mxs_lradc_reg_clear (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , -LRADC_CTRL0 ); - -if (lradc->use_touchscreen ==MXS_LRADC_TOUCHSCREEN_5WIRE ) -mxs_lradc_reg_set (lradc,LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE , -LRADC_CTRL0 ); - } -/* Start internal temperature sensing. */ mxs_lradc_reg_wrt (lradc,0 ,LRADC_CTRL2 );
Signature · 1 line
Dmitry
PATCH 2/5 iio: mxs-lradc: move TS config into suitable function
In reply to Jonathan CameronAm 17.04.2016 um 12:13 schrieb Jonathan Cameron:
Quoted 14 lines · Marek Vasut
Quoted 8 lines · Stefan Wahren
Quoted 4 lines
This patch moves the touchscreen type configuration into a more suitable function. Btw this simplifies PM ops later.
Signed-off-by Stefan Wahren <[email protected]>Looks OK
Reviewed-by Marek Vasut <[email protected]>Please do CC Dmitry and linux-input on any patches 'touching' the touchscreen parts of the driver. I'm happy this one is simple enough to apply anyway, but best to make sure everyone is in the loop.
Quoted 4 lines
Something a little odd happened when applying the patch. Automerging failed so I did it by hand. Not sure what had changed!
Anyhow, please sanity check I haven't messed it up.
Everything is fine. There was a indention conflict between Ksenija's and my patch.
Quoted 8 lines
Applied to the togreg branch of iio.git - initially pushed out as testing for the autobuilders to play with it.
Thanks,
Jonathan
PATCH 3/5 iio: mxs-lradc: move STMP reset out of ADC init
In reply to Stefan WahrenThis patch moves the STMP reset out of ADC init function in order to make it never fail. This is also necessary for PM ops.
--- drivers/iio/adc/mxs-lradc.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
drivers/iio/adc/mxs-lradc.c +5 -10
@@ -1482,18 +1482,13 @@ static const struct iio_chan_spec mx28_lradc_chan_spec[] = { MXS_ADC_CHAN(15, IIO_VOLTAGE, "VDD5V"), }; -static int mxs_lradc_hw_init (struct mxs_lradc *lradc) +static void mxs_lradc_hw_init (struct mxs_lradc *lradc) {/* The ADC always uses DELAY CHANNEL 0. */ const u32 adc_cfg = (1 << (LRADC_DELAY_TRIGGER_DELAYS_OFFSET +0 )) | (LRADC_DELAY_TIMER_PER <<LRADC_DELAY_DELAY_OFFSET ); -int ret =stmp_reset_block (lradc->base); - -if (ret) -return ret; -/* Configure DELAY CHANNEL 0 for generic ADC sampling. */ mxs_lradc_reg_wrt (lradc, adc_cfg,LRADC_DELAY (0 ));
@@ -1504,8 +1499,6 @@ static int mxs_lradc_hw_init(struct mxs_lradc *lradc)/* Start internal temperature sensing. */ mxs_lradc_reg_wrt (lradc,0 ,LRADC_CTRL2 ); - -return 0 ; }static void mxs_lradc_hw_stop (struct mxs_lradc *lradc)
@@ -1705,11 +1698,13 @@ static int mxs_lradc_probe(struct platform_device *pdev) } } -/* Configure the hardware. */ - ret =mxs_lradc_hw_init (lradc); + ret =stmp_reset_block (lradc->base);if (ret) goto err_dev; +/* Configure the hardware. */ +mxs_lradc_hw_init (lradc); +/* Register the touchscreen input device. */ if (touch_ret ==0 ) { ret =mxs_lradc_ts_register (lradc);
Signature · 1 line
1.7.9.5
PATCH 3/5 iio: mxs-lradc: move STMP reset out of ADC init
Hi Stefan,
Quoted 3 lines · On Thursday 14 April 2016 17:48:27 Stefan Wahren wrote:
This patch moves the STMP reset out of ADC init function in order to make it never fail. This is also necessary for PM ops.
What do you mean with "make it never fail"?
Quoted 1 line
[...]
jb
PATCH 3/5 iio: mxs-lradc: move STMP reset out of ADC init
Quoted 8 lines · Juergen Borleis
Hi Stefan,
Quoted 3 lines · On Thursday 14 April 2016 17:48:27 Stefan Wahren wrote:
This patch moves the STMP reset out of ADC init function in order to make it never fail. This is also necessary for PM ops.
What do you mean with "make it never fail"?
He wants to make the function invoke only functions which don't return error value. Not a great wording, but the patch makes it clear.
Quoted 4 lines
Quoted 1 line
[...]
jb
Signature · 2 lines
Best regards, Marek Vasut
Jonathan Cameron
maintainerPATCH 3/5 iio: mxs-lradc: move STMP reset out of ADC init
Quoted 4 lines · Marek Vasut
Quoted 3 lines · Juergen Borleis
Hi Stefan,
On Thursday 14 April 2016 17:48:27 Stefan Wahren wrote:
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
This patch moves the STMP reset out of ADC init function in order to make it never fail. This is also necessary for PM ops.
What do you mean with "make it never fail"?
He wants to make the function invoke only functions which don't return error value. Not a great wording, but the patch makes it clear.
I've reworded to make this a little clearer and applied it to the togreg branch of iio.git initially pushed out as testing for the autobuilders
to play with it (also giving time for any more comments on it to come through)
Jonathan
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
[...]
jb
PATCH 4/5 iio: mxs-lradc: disable only mapped channels in mxs_lradc_hw_stop
In reply to Stefan WahrenDisabling of the touchscreen IRQs is already done in mxs_lradc_disable_ts. There is no need to disable them in mxs_lradc_hw_stop again. So we only need to care of the mapped channels which are common for i.MX23 and i.MX28 and we remove the now unused function mxs_lradc_irq_en_mask.
--- drivers/iio/adc/mxs-lradc.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
drivers/iio/adc/mxs-lradc.c +3 -8
@@ -295,6 +295,7 @@ struct mxs_lradc {#define LRADC_CTRL1_LRADC_IRQ_EN (n) (1 << ((n) + 16))#define LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK (0x1fff << 16)#define LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK (0x01ff << 16) +#define LRADC_CTRL1_MAPPED_CHANS_IRQ_EN_MASK (0x00ff << 16)#define LRADC_CTRL1_LRADC_IRQ_EN_OFFSET 16#define LRADC_CTRL1_TOUCH_DETECT_IRQ BIT(8)#define LRADC_CTRL1_LRADC_IRQ (n) (1 << (n))
@@ -373,13 +374,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 +1499,8 @@ 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_CTRL1_MAPPED_CHANS_IRQ_EN_MASK , +LRADC_CTRL1 );for (i =0 ; i <LRADC_MAX_DELAY_CHANS ; i++)mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (i));
Signature · 1 line
1.7.9.5
Jonathan Cameron
maintainerPATCH 4/5 iio: mxs-lradc: disable only mapped channels in mxs_lradc_hw_stop
Quoted 3 lines · Stefan Wahren
Disabling of the touchscreen IRQs is already done in
mxs_lradc_disable_ts. There is no need to disable them inmxs_lradc_hw_stopagain.
We could do with a little more documentation in the driver on which bits in this register are doing what.
The disable_ts deals with bits 22-24...
The only other bit I think is ever used by the driver currently is 16 (channel 0).
So whilst this change probably works, I'm not sure of the logic behind it. As far as I can tell your new clear is fine but in reality you could just clear (0x3f << 16) and get the same result (which would be clearer to my mind).
The key think here is that we don't support the higher bits on i.mx28 yet... Which are for button detection and threshold detection.
Have I understood this correctly?
Jonathan
So we only need to care of the
Quoted 45 lines
mapped channels which are common for i.MX23 and i.MX28 and we remove the now unused function
mxs_lradc_irq_en_mask.Signed-off-by Stefan Wahren <[email protected]>--- drivers/iio/adc/mxs-lradc.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)drivers/iio/adc/mxs-lradc.c +3
-8@@ -295,6 +295,7 @@ struct mxs_lradc {#define LRADC_CTRL1_LRADC_IRQ_EN (n) (1 << ((n) + 16))#define LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK (0x1fff << 16)#define LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK (0x01ff << 16) +#define LRADC_CTRL1_MAPPED_CHANS_IRQ_EN_MASK (0x00ff << 16)#define LRADC_CTRL1_LRADC_IRQ_EN_OFFSET 16#define LRADC_CTRL1_TOUCH_DETECT_IRQ BIT(8)#define LRADC_CTRL1_LRADC_IRQ (n) (1 << (n))@@ -373,13 +374,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 +1499,8 @@ 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_CTRL1_MAPPED_CHANS_IRQ_EN_MASK , +LRADC_CTRL1 );for (i =0 ; i <LRADC_MAX_DELAY_CHANS ; i++)mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (i));
PATCH 4/5 iio: mxs-lradc: disable only mapped channels in mxs_lradc_hw_stop
Hi Jonathan,
Am 17.04.2016 um 12:47 schrieb Jonathan Cameron:
Quoted 16 lines · Stefan Wahren
Quoted 3 lines
Disabling of the touchscreen IRQs is already done in
mxs_lradc_disable_ts. There is no need to disable them inmxs_lradc_hw_stopagain.We could do with a little more documentation in the driver on which bits in this register are doing what.
The
disable_tsdeals with bits 22-24...
The only other bit I think is ever used by the driver currently is 16 (channel 0).So whilst this change probably works, I'm not sure of the logic behind it. As far as I can tell your new clear is fine but in reality you could just clear (
0x3f<< 16) and get the same result (which would be clearer to my mind).
Quoted 4 lines
The key think here is that we don't support the higher bits on i.mx28 yet... Which are for button detection and threshold detection.
Have I understood this correctly?
Yes, you are. I've have looked to long in the reference manual instead of the code.
I've missed the point that virtual channel 6 and 7 are used in case of a connected touchscreen.
I should use the member buffer_vchans for masking the enable IRQs.
Regards
Stefan
Quoted 3 lines
Jonathan
So we only need to care of the
PATCH 5/5 iio: mxs-lradc: implement suspend/resume support
In reply to Stefan WahrenThis patch implements suspend/resume support for mxs-lradc. It's possible to use the touchscreen as wakeup source.
--- drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
drivers/iio/adc/mxs-lradc.c +61 -0
@@ -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,
Signature · 1 line
1.7.9.5
Jonathan Cameron
maintainerPATCH 5/5 iio: mxs-lradc: implement suspend/resume support
Quoted 4 lines · 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 <[email protected]>
Looks good to me pending replies to the previous patch comments.
Quoted 88 lines
--- drivers/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)drivers/iio/adc/mxs-lradc.c +61
-0@@ -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,
cover iio: mxs-lradc: fix leak and implement PM ops
In reply to Stefan WahrenQuoted 19 lines · Stefan Wahren
This patch series fix a memory leak and implements suspend/resume support for mxs-lradc.
Patch 1: fix the memory leak
Patch 2-4: refactor mxs-lradc in order to prepare PM implementation Patch 5: implement suspend/resume supportThese 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.
Stefan Wahren (5):
iio: mxs-lradc: fix memory leak
iio: mxs-lradc: move TS config into suitable function
iio: mxs-lradc: move STMP reset out of ADC init
iio: mxs-lradc: disable only mapped channels inmxs_lradc_hw_stop
iio: mxs-lradc: implement suspend/resume support
Quoted 3 lines
drivers/iio/adc/mxs-lradc.c | 116 ++++++++++++++++++++++++++++++------------- 1 file changed, 82 insertions(+), 34 deletions(-)
On M28EVK:
Signature · 2 lines
Best regards, Marek Vasut
cover iio: mxs-lradc: fix leak and implement PM ops
Am 14.04.2016 um 21:55 schrieb Marek Vasut:
Quoted 20 lines · Stefan Wahren
Quoted 19 lines
This patch series fix a memory leak and implements suspend/resume support for mxs-lradc.
Patch 1: fix the memory leak
Patch 2-4: refactor mxs-lradc in order to prepare PM implementation Patch 5: implement suspend/resume supportThese 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.
Stefan Wahren (5):
iio: mxs-lradc: fix memory leak
iio: mxs-lradc: move TS config into suitable function
iio: mxs-lradc: move STMP reset out of ADC init
iio: mxs-lradc: disable only mapped channels inmxs_lradc_hw_stop
iio: mxs-lradc: implement suspend/resume support
Quoted 7 lines
Quoted 3 lines
drivers/iio/adc/mxs-lradc.c | 116 ++++++++++++++++++++++++++++++------------- 1 file changed, 82 insertions(+), 34 deletions(-)On M28EVK:
Tested-by Marek Vasut <[email protected]>
Thanks. Only one question, does it imply a test with a touchscreen?
cover iio: mxs-lradc: fix leak and implement PM ops
Quoted 24 lines · Stefan Wahren
Am 14.04.2016 um 21:55 schrieb Marek Vasut:
Quoted 23 lines · Stefan Wahren
Quoted 22 lines
This patch series fix a memory leak and implements suspend/resume support for mxs-lradc.
Patch 1: fix the memory leak
Patch 2-4: refactor mxs-lradc in order to prepare PM implementation Patch 5: implement suspend/resume supportThese 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.
Stefan Wahren (5):
iio: mxs-lradc: fix memory leak
iio: mxs-lradc: move TS config into suitable function
iio: mxs-lradc: move STMP reset out of ADC init
iio: mxs-lradc: disable only mapped channels inmxs_lradc_hw_stop
iio: mxs-lradc: implement suspend/resume supportdrivers/iio/adc/mxs-lradc.c | 116 ++++++++++++++++++++++++++++++------------- 1 file changed, 82 insertions(+), 34 deletions(-)
Quoted 8 lines
Quoted 4 lines
On M28EVK:
Tested-by Marek Vasut <[email protected]>Thanks. Only one question, does it imply a test with a touchscreen?
Yes, this one [1]. It's an Ampire 800480 display and resistive touch on top of it.
[1] http://www.denx-cs.de/?q=MxxDK
Signature · 2 lines
Best regards, Marek Vasut