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