Re: [PATCH] fbdev: bfin-lq035q1-fb: Use dev_pm_ops

[Lars-Peter Clausen <lars@metafoo.de>]

On 05/30/2013 12:45 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 09:52 Thu 30 May     , Lars-Peter Clausen wrote:
>> On 05/30/2013 09:14 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 09:32 Thu 30 May     , Jingoo Han wrote:
>>>> On Thursday, May 30, 2013 4:20 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>> On 16:24 Wed 29 May     , Michael Hennerich wrote:
>>>>>> On 05/29/2013 02:17 PM, Lars-Peter Clausen wrote:
>>>>>>> Use dev_pm_ops instead of the legacy suspend/resume callbacks.
>>>>>>>
>>>>>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>>>>>> Acked-by: Michael Hennerich <michael.hennerich@analog.com>
>>>>>>> ---
>>>>>>>  drivers/video/bfin-lq035q1-fb.c | 22 ++++++++++++++--------
>>>>>>>  1 file changed, 14 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/video/bfin-lq035q1-fb.c b/drivers/video/bfin-lq035q1-fb.c
>>>>>>> index 29d8c04..6084c17 100644
>>>>>>> --- a/drivers/video/bfin-lq035q1-fb.c
>>>>>>> +++ b/drivers/video/bfin-lq035q1-fb.c
>>>>>>> @@ -170,16 +170,19 @@ static int lq035q1_spidev_remove(struct spi_device *spi)
>>>>>>>  	return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_SHUT);
>>>>>>>  }
>>>>>>> -#ifdef CONFIG_PM
>>>>>>> -static int lq035q1_spidev_suspend(struct spi_device *spi, pm_message_t state)
>>>>>>> +#ifdef CONFIG_PM_SLEEP
>>>>>>> +static int lq035q1_spidev_suspend(struct device *dev)
>>>>>>>  {
>>>>>>> +	struct spi_device *spi = to_spi_device(dev);
>>>>>>> +
>>>>>>>  	return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_SHUT);
>>>>>>>  }
>>>>>>> -static int lq035q1_spidev_resume(struct spi_device *spi)
>>>>>>> +static int lq035q1_spidev_resume(struct device *dev)
>>>>>>>  {
>>>>>>> -	int ret;
>>>>>>> +	struct spi_device *spi = to_spi_device(dev);
>>>>>>>  	struct spi_control *ctl = spi_get_drvdata(spi);
>>>>>>> +	int ret;
>>>>>>>  	ret = lq035q1_control(spi, LQ035_DRIVER_OUTPUT_CTL, ctl->mode);
>>>>>>>  	if (ret)
>>>>>>> @@ -187,9 +190,13 @@ static int lq035q1_spidev_resume(struct spi_device *spi)
>>>>>>>  	return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_ON);
>>>>>>>  }
>>>>>>> +
>>>>>>> +static SIMPLE_DEV_PM_OPS(lq035q1_spidev_pm_ops, lq035q1_spidev_suspend,
>>>>>>> +	lq035q1_spidev_resume);
>>>>>>> +#define LQ035Q1_SPIDEV_PM_OPS (&lq035q1_spidev_pm_ops)
>>>>>>> +
>>>>>>>  #else
>>>>>>> -# define lq035q1_spidev_suspend NULL
>>>>>>> -# define lq035q1_spidev_resume  NULL
>>>>>>> +#define LQ035Q1_SPIDEV_PM_OPS NULL
>>>>>>>  #endif
>>>>> we really need to ahve a macro like for DT of_match_ptr to drop the #else
>>>>
>>>> Hi Jean-Christophe PLAGNIOL-VILLARD,
>>>>
>>>> I submitted the following patch. :)
>>>> (https://patchwork.kernel.org/patch/2502971/)
>>>>
>>>> --- a/include/linux/pm.h
>>>> +++ b/include/linux/pm.h
>>>> @@ -55,8 +55,10 @@  struct device;
>>>>  
>>>>  #ifdef CONFIG_PM
>>>>  extern const char power_group_name[];		/* = "power" */
>>>> +#define pm_ops_ptr(_ptr)	(_ptr)
>>>>  #else
>>>>  #define power_group_name	NULL
>>>> +#define pm_ops_ptr(_ptr)	NULL
>>>>  #endif
>>>>
>>>>
>>>> This patch was accepted by Rafael Wysocki, and will be merged to v3.11-rc1.
>>>>
>>> Lars-Peter please update with and
>>
>> Since the code depends on CONFIG_PM_SLEEP and not CONFIG_PM I don't think
>> the macro will work.
> 
> se please ad a similar macros too

I'm not really convinced that this *_ops_ptr thing is the right approach.
Besides that the scope of this patch is to convert the driver from legacy
suspend callbacks to dev_pm_ops not to remove the #ifdef's.

- Lars