Re: [PATCH] clk: vc5: Add suspend/resume support

[Marek Vasut <marek.vasut@gmail.com>]

On 12/05/2018 06:21 AM, Laurent Pinchart wrote:
> Hi Marek,
> 
> On Wednesday, 5 December 2018 01:48:01 EET Marek Vasut wrote:
>> On 12/04/2018 09:52 PM, Stephen Boyd wrote:
>>> Quoting Marek Vasut (2018-12-04 10:27:21)
>>>
>>>> diff --git a/drivers/clk/clk-versaclock5.c
>>>> b/drivers/clk/clk-versaclock5.c
>>>> index decffb3826ec..ac90fb36af1a 100644
>>>> --- a/drivers/clk/clk-versaclock5.c
>>>> +++ b/drivers/clk/clk-versaclock5.c
>>>> @@ -906,6 +906,39 @@ static int vc5_remove(struct i2c_client *client)
>>>>
>>>>         return 0;
>>>>  
>>>>  }
>>>>
>>>> +#ifdef CONFIG_PM_SLEEP
>>>> +static int vc5_suspend(struct device *dev)
>>>
>>> Please mark as __maybe_unused and drop the #ifdef CONFIG_PM_SLEEP
>>>
>>>> +{
>>>> +       struct vc5_driver_data *vc5 = dev_get_drvdata(dev);
>>>> +       int ret;
>>>> +
>>>> +       ret = regcache_sync(vc5->regmap);
>>>> +       if (ret != 0) {
>>>> +               dev_err(dev, "Failed to save register map: %d\n", ret);
>>>> +               return ret;
>>>
>>> Do we need to block suspend if we can't save the register map away? Or
>>> can we just throw up our hands and not restore on resume?
>>
>> Some hardware will fail on resume, so I'd say -- yes ?
> 
> But why do you need to sync on suspend in the first place ? What could cause 
> the map to be dirty at this stage, and require syncing before suspend, that 
> couldn't work with the sync be delayed to resume time ?

Possibly a configuration coming from eg. bootloader time , or some other
configuration not done by Linux.

-- 
Best regards,
Marek Vasut