2017-06-22 12:49:43

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH] iio: adc: at91-sama5d2_adc: add support for suspend/resume functionality

Added support for suspend/resume functionality for the ADC IP
in sama5d2 SoC.
In order to enter Suspend to ram mode (backup + self refresh mode for
memory), in which the ADC IP is no longer powered, we need to reset the
pins to default state, for the scenario when they are also used for I2C
bus to communicate with the PMIC.
On resume, we need to reconfigure the ADC IP registers and reconfigure the
trigger registers in the case when the suspend procedure is done while
sysfs has the buffer and trigger enabled.
In the case the suspend happens exactly during a software triggered
conversion, the request will time out, because we reset and power down
the ADC.

Signed-off-by: Eugen Hristev <[email protected]>
---
drivers/iio/adc/at91-sama5d2_adc.c | 91 +++++++++++++++++++++++++++++++++-----
1 file changed, 81 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index 1773a5d..3e775c7 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -576,6 +576,20 @@ static const struct iio_info at91_adc_info = {
.driver_module = THIS_MODULE,
};

+static void at91_adc_hw_init(struct at91_adc_state *st)
+{
+ at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_SWRST);
+ at91_adc_writel(st, AT91_SAMA5D2_IDR, 0xffffffff);
+ /*
+ * Transfer field must be set to 2 according to the datasheet and
+ * allows different analog settings for each channel.
+ */
+ at91_adc_writel(st, AT91_SAMA5D2_MR,
+ AT91_SAMA5D2_MR_TRANSFER(2) | AT91_SAMA5D2_MR_ANACH);
+
+ at91_adc_setup_samp_freq(st, st->soc_info.min_sample_rate);
+}
+
static int at91_adc_probe(struct platform_device *pdev)
{
struct iio_dev *indio_dev;
@@ -694,16 +708,7 @@ static int at91_adc_probe(struct platform_device *pdev)
goto vref_disable;
}

- at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_SWRST);
- at91_adc_writel(st, AT91_SAMA5D2_IDR, 0xffffffff);
- /*
- * Transfer field must be set to 2 according to the datasheet and
- * allows different analog settings for each channel.
- */
- at91_adc_writel(st, AT91_SAMA5D2_MR,
- AT91_SAMA5D2_MR_TRANSFER(2) | AT91_SAMA5D2_MR_ANACH);
-
- at91_adc_setup_samp_freq(st, st->soc_info.min_sample_rate);
+ at91_adc_hw_init(st);

ret = clk_prepare_enable(st->per_clk);
if (ret)
@@ -759,6 +764,71 @@ static int at91_adc_remove(struct platform_device *pdev)
return 0;
}

+#ifdef CONFIG_PM_SLEEP
+static int at91_adc_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev =
+ platform_get_drvdata(to_platform_device(dev));
+ struct at91_adc_state *st = iio_priv(indio_dev);
+
+ /*
+ * Do a sofware reset of the ADC before we go to suspend.
+ * this will ensure that all pins are free from being muxed by the ADC
+ * and can be used by for other devices.
+ * Otherwise, ADC will hog them and we can't go to suspend mode.
+ */
+ at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_SWRST);
+
+ clk_disable_unprepare(st->per_clk);
+ regulator_disable(st->vref);
+ regulator_disable(st->reg);
+
+ return pinctrl_pm_select_sleep_state(dev);
+}
+
+static int at91_adc_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev =
+ platform_get_drvdata(to_platform_device(dev));
+ struct at91_adc_state *st = iio_priv(indio_dev);
+ int ret;
+
+ ret = pinctrl_pm_select_default_state(dev);
+ if (ret)
+ goto resume_failed;
+
+ ret = regulator_enable(st->reg);
+ if (ret)
+ goto resume_failed;
+
+ ret = regulator_enable(st->vref);
+ if (ret)
+ goto reg_disable_resume;
+
+ ret = clk_prepare_enable(st->per_clk);
+ if (ret)
+ goto vref_disable_resume;
+
+ at91_adc_hw_init(st);
+
+ /* reconfiguring trigger hardware state */
+ if (iio_buffer_enabled(indio_dev))
+ at91_adc_configure_trigger(st->trig, true);
+
+ return 0;
+
+vref_disable_resume:
+ regulator_disable(st->vref);
+reg_disable_resume:
+ regulator_disable(st->reg);
+resume_failed:
+ dev_err(&indio_dev->dev, "failed to resume\n");
+ return ret;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(at91_adc_pm_ops, at91_adc_suspend, at91_adc_resume);
+
static const struct of_device_id at91_adc_dt_match[] = {
{
.compatible = "atmel,sama5d2-adc",
@@ -774,6 +844,7 @@ static struct platform_driver at91_adc_driver = {
.driver = {
.name = "at91-sama5d2_adc",
.of_match_table = at91_adc_dt_match,
+ .pm = &at91_adc_pm_ops,
},
};
module_platform_driver(at91_adc_driver)
--
2.7.4


2017-06-22 22:07:53

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] iio: adc: at91-sama5d2_adc: add support for suspend/resume functionality

Hi Eugen,

[auto build test ERROR on iio/togreg]
[also build test ERROR on v4.12-rc6 next-20170622]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Eugen-Hristev/iio-adc-at91-sama5d2_adc-add-support-for-suspend-resume-functionality/20170623-043330
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64

All errors (new ones prefixed by >>):

drivers/iio/adc/at91-sama5d2_adc.c: In function 'at91_adc_suspend':
>> drivers/iio/adc/at91-sama5d2_adc.c:559:9: error: implicit declaration of function 'pinctrl_pm_select_sleep_state' [-Werror=implicit-function-declaration]
return pinctrl_pm_select_sleep_state(dev);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iio/adc/at91-sama5d2_adc.c: In function 'at91_adc_resume':
>> drivers/iio/adc/at91-sama5d2_adc.c:569:8: error: implicit declaration of function 'pinctrl_pm_select_default_state' [-Werror=implicit-function-declaration]
ret = pinctrl_pm_select_default_state(dev);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/iio/adc/at91-sama5d2_adc.c:589:3: error: implicit declaration of function 'at91_adc_configure_trigger' [-Werror=implicit-function-declaration]
at91_adc_configure_trigger(st->trig, true);
^~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/iio/adc/at91-sama5d2_adc.c:589:32: error: 'struct at91_adc_state' has no member named 'trig'; did you mean 'reg'?
at91_adc_configure_trigger(st->trig, true);
^~
cc1: some warnings being treated as errors

vim +/pinctrl_pm_select_sleep_state +559 drivers/iio/adc/at91-sama5d2_adc.c

553 at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_SWRST);
554
555 clk_disable_unprepare(st->per_clk);
556 regulator_disable(st->vref);
557 regulator_disable(st->reg);
558
> 559 return pinctrl_pm_select_sleep_state(dev);
560 }
561
562 static int at91_adc_resume(struct device *dev)
563 {
564 struct iio_dev *indio_dev =
565 platform_get_drvdata(to_platform_device(dev));
566 struct at91_adc_state *st = iio_priv(indio_dev);
567 int ret;
568
> 569 ret = pinctrl_pm_select_default_state(dev);
570 if (ret)
571 goto resume_failed;
572
573 ret = regulator_enable(st->reg);
574 if (ret)
575 goto resume_failed;
576
577 ret = regulator_enable(st->vref);
578 if (ret)
579 goto reg_disable_resume;
580
581 ret = clk_prepare_enable(st->per_clk);
582 if (ret)
583 goto vref_disable_resume;
584
585 at91_adc_hw_init(st);
586
587 /* reconfiguring trigger hardware state */
588 if (iio_buffer_enabled(indio_dev))
> 589 at91_adc_configure_trigger(st->trig, true);
590
591 return 0;
592

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.25 kB)
.config.gz (49.51 kB)
Download all attachments

2017-06-23 09:26:52

by Eugen Hristev

[permalink] [raw]
Subject: Re: [PATCH] iio: adc: at91-sama5d2_adc: add support for suspend/resume functionality

On 23.06.2017 01:07, kbuild test robot wrote:
> Hi Eugen,
>
> [auto build test ERROR on iio/togreg]
> [also build test ERROR on v4.12-rc6 next-20170622]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Eugen-Hristev/iio-adc-at91-sama5d2_adc-add-support-for-suspend-resume-functionality/20170623-043330
> base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> config: sparc64-allyesconfig (attached as .config)
> compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=sparc64
>
> All errors (new ones prefixed by >>):
>
> drivers/iio/adc/at91-sama5d2_adc.c: In function 'at91_adc_suspend':
>>> drivers/iio/adc/at91-sama5d2_adc.c:559:9: error: implicit declaration of function 'pinctrl_pm_select_sleep_state' [-Werror=implicit-function-declaration]
> return pinctrl_pm_select_sleep_state(dev);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Will send a revised patch to fix this warning on SPARC architecture.

> drivers/iio/adc/at91-sama5d2_adc.c: In function 'at91_adc_resume':
>>> drivers/iio/adc/at91-sama5d2_adc.c:569:8: error: implicit declaration of function 'pinctrl_pm_select_default_state' [-Werror=implicit-function-declaration]
> ret = pinctrl_pm_select_default_state(dev);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/iio/adc/at91-sama5d2_adc.c:589:3: error: implicit declaration of function 'at91_adc_configure_trigger' [-Werror=implicit-function-declaration]
> at91_adc_configure_trigger(st->trig, true);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/iio/adc/at91-sama5d2_adc.c:589:32: error: 'struct at91_adc_state' has no member named 'trig'; did you mean 'reg'?
> at91_adc_configure_trigger(st->trig, true);
These two errors appears because this patch is on top of my other patch
for hardware trigger and buffer which is on the way upstream, and not
yet in linux-next.
Building over the tree with the included patch will work.

Regards,
Eugen
> ^~
> cc1: some warnings being treated as errors
>
> vim +/pinctrl_pm_select_sleep_state +559 drivers/iio/adc/at91-sama5d2_adc.c
>
> 553 at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_SWRST);
> 554
> 555 clk_disable_unprepare(st->per_clk);
> 556 regulator_disable(st->vref);
> 557 regulator_disable(st->reg);
> 558
> > 559 return pinctrl_pm_select_sleep_state(dev);
> 560 }
> 561
> 562 static int at91_adc_resume(struct device *dev)
> 563 {
> 564 struct iio_dev *indio_dev =
> 565 platform_get_drvdata(to_platform_device(dev));
> 566 struct at91_adc_state *st = iio_priv(indio_dev);
> 567 int ret;
> 568
> > 569 ret = pinctrl_pm_select_default_state(dev);
> 570 if (ret)
> 571 goto resume_failed;
> 572
> 573 ret = regulator_enable(st->reg);
> 574 if (ret)
> 575 goto resume_failed;
> 576
> 577 ret = regulator_enable(st->vref);
> 578 if (ret)
> 579 goto reg_disable_resume;
> 580
> 581 ret = clk_prepare_enable(st->per_clk);
> 582 if (ret)
> 583 goto vref_disable_resume;
> 584
> 585 at91_adc_hw_init(st);
> 586
> 587 /* reconfiguring trigger hardware state */
> 588 if (iio_buffer_enabled(indio_dev))
> > 589 at91_adc_configure_trigger(st->trig, true);
> 590
> 591 return 0;
> 592
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>

2017-06-24 19:52:39

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] iio: adc: at91-sama5d2_adc: add support for suspend/resume functionality

On Fri, 23 Jun 2017 12:25:35 +0300
Eugen Hristev <[email protected]> wrote:

> On 23.06.2017 01:07, kbuild test robot wrote:
> > Hi Eugen,
> >
> > [auto build test ERROR on iio/togreg]
> > [also build test ERROR on v4.12-rc6 next-20170622]
> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> >
> > url: https://github.com/0day-ci/linux/commits/Eugen-Hristev/iio-adc-at91-sama5d2_adc-add-support-for-suspend-resume-functionality/20170623-043330
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> > config: sparc64-allyesconfig (attached as .config)
> > compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> > reproduce:
> > wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # save the attached .config to linux build tree
> > make.cross ARCH=sparc64
> >
> > All errors (new ones prefixed by >>):
> >
> > drivers/iio/adc/at91-sama5d2_adc.c: In function 'at91_adc_suspend':
> >>> drivers/iio/adc/at91-sama5d2_adc.c:559:9: error: implicit declaration of function 'pinctrl_pm_select_sleep_state' [-Werror=implicit-function-declaration]
> > return pinctrl_pm_select_sleep_state(dev);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> Will send a revised patch to fix this warning on SPARC architecture.
>
> > drivers/iio/adc/at91-sama5d2_adc.c: In function 'at91_adc_resume':
> >>> drivers/iio/adc/at91-sama5d2_adc.c:569:8: error: implicit declaration of function 'pinctrl_pm_select_default_state' [-Werror=implicit-function-declaration]
> > ret = pinctrl_pm_select_default_state(dev);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>> drivers/iio/adc/at91-sama5d2_adc.c:589:3: error: implicit declaration of function 'at91_adc_configure_trigger' [-Werror=implicit-function-declaration]
> > at91_adc_configure_trigger(st->trig, true);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~
> >>> drivers/iio/adc/at91-sama5d2_adc.c:589:32: error: 'struct at91_adc_state' has no member named 'trig'; did you mean 'reg'?
> > at91_adc_configure_trigger(st->trig, true);
> These two errors appears because this patch is on top of my other patch
> for hardware trigger and buffer which is on the way upstream, and not
> yet in linux-next.
> Building over the tree with the included patch will work.
Ah that explains the cryptic comment in the v2 of this patch.
Good to have that cleared up!
>
> Regards,
> Eugen
> > ^~
> > cc1: some warnings being treated as errors
> >
> > vim +/pinctrl_pm_select_sleep_state +559 drivers/iio/adc/at91-sama5d2_adc.c
> >
> > 553 at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_SWRST);
> > 554
> > 555 clk_disable_unprepare(st->per_clk);
> > 556 regulator_disable(st->vref);
> > 557 regulator_disable(st->reg);
> > 558
> > > 559 return pinctrl_pm_select_sleep_state(dev);
> > 560 }
> > 561
> > 562 static int at91_adc_resume(struct device *dev)
> > 563 {
> > 564 struct iio_dev *indio_dev =
> > 565 platform_get_drvdata(to_platform_device(dev));
> > 566 struct at91_adc_state *st = iio_priv(indio_dev);
> > 567 int ret;
> > 568
> > > 569 ret = pinctrl_pm_select_default_state(dev);
> > 570 if (ret)
> > 571 goto resume_failed;
> > 572
> > 573 ret = regulator_enable(st->reg);
> > 574 if (ret)
> > 575 goto resume_failed;
> > 576
> > 577 ret = regulator_enable(st->vref);
> > 578 if (ret)
> > 579 goto reg_disable_resume;
> > 580
> > 581 ret = clk_prepare_enable(st->per_clk);
> > 582 if (ret)
> > 583 goto vref_disable_resume;
> > 584
> > 585 at91_adc_hw_init(st);
> > 586
> > 587 /* reconfiguring trigger hardware state */
> > 588 if (iio_buffer_enabled(indio_dev))
> > > 589 at91_adc_configure_trigger(st->trig, true);
> > 590
> > 591 return 0;
> > 592
> >
> > ---
> > 0-DAY kernel test infrastructure Open Source Technology Center
> > https://lists.01.org/pipermail/kbuild-all Intel Corporation
> >