Hi Sakari, Thanks for the review. My comments below. --- ^Divagar >-----Original Message----- >From: Sakari Ailus [mailto:sakari.ailus@xxxxxx] >Sent: Wednesday, August 30, 2017 6:11 PM >To: Mohandass, Divagar <divagar.mohandass@xxxxxxxxx> >Cc: robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; wsa@xxxxxxxxxxxxx; >devicetree@xxxxxxxxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; linux- >kernel@xxxxxxxxxxxxxxx; Mani, Rajmohan <rajmohan.mani@xxxxxxxxx> >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.ailus@xxxxxx -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
