Hi Sakari,

Thanks for the review.
My comments below.

---
^Divagar

-----Original Message-----
From: Sakari Ailus [mailto:sakari.ailus@iki.fi]
Sent: Wednesday, August 30, 2017 6:11 PM
To: Mohandass, Divagar <redacted>
Cc: robh+dt@kernel.org; mark.rutland@arm.com; wsa@the-dreams.de;
devicetree@vger.kernel.org; linux-i2c@vger.kernel.org; linux-
kernel@vger.kernel.org; Mani, Rajmohan [off-list ref]
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:

quoted

quoted

quoted

@@ -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@iki.fi