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
