Hi Sakari, Thanks for the review. My comments below.
--- ^Divagar >-----Original Message----- >From: Sakari Ailus [mailto:sakari.ai...@iki.fi] >Sent: Wednesday, August 30, 2017 6:11 PM >To: Mohandass, Divagar <divagar.mohand...@intel.com> >Cc: robh...@kernel.org; mark.rutl...@arm.com; w...@the-dreams.de; >devicet...@vger.kernel.org; linux-...@vger.kernel.org; linux- >ker...@vger.kernel.org; Mani, Rajmohan <rajmohan.m...@intel.com> >Subject: Re: [PATCH v3 3/3] eeprom: at24: enable runtime pm support > >On Wed, Aug 30, 2017 at 12:32:07PM +0000, Mohandass, Divagar wrote: >> >> @@ -743,6 +770,15 @@ static int at24_probe(struct i2c_client >> >> *client, const struct i2c_device_id *id) >> >> >> >> i2c_set_clientdata(client, at24); >> >> >> >> + /* enable runtime pm */ >> >> + pm_runtime_get_noresume(&client->dev); >> >> + err = pm_runtime_set_active(&client->dev); >> >> + if (err < 0) >> >> + goto err_clients; >> >> + >> >> + pm_runtime_enable(&client->dev); >> >> + pm_runtime_put(&client->dev); >> >> + >> > >> >You're just about to perform a read here. I believe you should move >> >the last put after that. >> >> At the end of at24_read we are performing a pm_runtime_put, still we need >this change ? > >True, so this isn't an actual problem. > >It'll still power the chip down when you're about to need it, so it'd make >sense >to perform the check before pm_runtime_put(). > >I might move the runtime PM setup after the check altogether. Ok, I will move the pm_runtime_put() after the check and publish the v4. Moving the PM setup altogether below, will introduce more error handling in read call. > >-- >Sakari Ailus >e-mail: sakari.ai...@iki.fi
