Hi Dmitry, On 20-01-10 08:18, Marco Felsch wrote: > On 20-01-10 08:16, Marco Felsch wrote: > > Hi Dmitry, > > > > On 20-01-09 17:09, Dmitry Torokhov wrote: > > > Hi Marco, > > > > > > On Wed, Jan 08, 2020 at 12:10:50PM +0100, Marco Felsch wrote: > > > > +static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev) > > > > +{ > > > > + struct i2c_client *client = to_i2c_client(dev); > > > > + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client); > > > > + int ret; > > > > + > > > > + if (device_may_wakeup(dev)) > > > > + return 0; > > > > + > > > > + ret = regulator_enable(tsdata->vcc); > > > > + if (ret) > > > > + dev_warn(dev, "Failed to enable vcc\n"); > > > > > > I wonder if we should not return error here instead of continuing. If > > > device is not powered up properly we'll have hard time communicating > > > with it. > > > > That's a reasonable point. > > > > > The same is for suspend: maybe we should abort if we can't switch off > > > regulator or write to the device. > > > > I have no strong opinion about that case but IMHO it's okay to go further > > if we can't switch it off. Instead we should print a warning. > > I just noticed that we do that already.. So the suspend case should be > okay. Is it okay to check the return val for the resume case only? I want to prepare a v4 of this patch to get this done. Regards, Marco > > > Regards, > > Marco > > > > > Thanks. > > > > > > -- > > > Dmitry > > > > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |