[PATCH RFT V2 3/3] iio: mxs-lradc: implement suspend/resume support

[Marek Vasut]

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 at 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 at denx.de>
>>
> 
> 


-- 
Best regards,
Marek Vasut