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

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

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 ?

The rest is fixed.

>> +       }
>> +       regcache_cache_only(vc5->regmap, true);
>> +       regcache_mark_dirty(vc5->regmap);
>> +
>> +       return 0;
>> +}
>> +
>> +static int vc5_resume(struct device *dev)
>> +{
>> +       struct vc5_driver_data *vc5 = dev_get_drvdata(dev);
>> +       int ret;
>> +
>> +       regcache_cache_only(vc5->regmap, false);
>> +       ret = regcache_sync(vc5->regmap);
>> +       if (ret != 0) {
>> +               dev_err(dev, "Failed to restore register map: %d\n", ret);
>> +               return ret;
>> +       }
> 
> Simplify to
> 
> 	if (ret)
> 		dev_err()
> 	retun ret;
> 
>> +
>> +       return 0;
>> +}
>> +#endif
>> +
>>  static const struct vc5_chip_info idt_5p49v5923_info = {
>>         .model = IDT_VC5_5P49V5923,
>>         .clk_fod_cnt = 2,


-- 
Best regards,
Marek Vasut