Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752292AbdIAS0y convert rfc822-to-8bit (ORCPT ); Fri, 1 Sep 2017 14:26:54 -0400 Received: from mga05.intel.com ([192.55.52.43]:14939 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750955AbdIAS0w (ORCPT ); Fri, 1 Sep 2017 14:26:52 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,459,1498546800"; d="scan'208";a="1213572470" From: "Mohandass, Divagar" To: Sakari Ailus 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" Subject: RE: [PATCH v4 3/3] eeprom: at24: enable runtime pm support Thread-Topic: [PATCH v4 3/3] eeprom: at24: enable runtime pm support Thread-Index: AQHTIbJFKrbse1a78EGs9xy2tFII6qKdC8cAgAFEx2D//+3VAIABsqDw Date: Fri, 1 Sep 2017 18:26:47 +0000 Message-ID: <7B8CE47BD58441468D2BB13285B50E6031DE74AB@BGSMSX107.gar.corp.intel.com> References: <1504112740-14072-1-git-send-email-divagar.mohandass@intel.com> <1504112740-14072-4-git-send-email-divagar.mohandass@intel.com> <20170830211938.3m4xdwlfq5yozymq@valkosipuli.retiisi.org.uk> <7B8CE47BD58441468D2BB13285B50E6031DE6BFC@BGSMSX107.gar.corp.intel.com> <20170831153702.eeilqwdxsoy4e3vt@valkosipuli.retiisi.org.uk> In-Reply-To: <20170831153702.eeilqwdxsoy4e3vt@valkosipuli.retiisi.org.uk> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.223.10.10] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2123 Lines: 70 Hi Sakari, Thanks for the review. My comments below. --- ^Divagar >-----Original Message----- >From: Sakari Ailus [mailto:sakari.ailus@iki.fi] >Sent: Thursday, August 31, 2017 9:07 PM >To: Mohandass, Divagar >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 >Subject: Re: [PATCH v4 3/3] eeprom: at24: enable runtime pm support > >On Thu, Aug 31, 2017 at 11:24:38AM +0000, Mohandass, Divagar wrote: >> >> @@ -743,6 +770,14 @@ 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; >> > >> >Btw. I don't think pm_runtime_set_active() can fail here. In other >> >words it'd be fine to ignore the return value. >> > >> >> Ack >> >> >> >> + >> >> + pm_runtime_enable(&client->dev); >> >> + >> >> /* >> >> * Perform a one-byte test read to verify that the >> >> * chip is functional. >> >> @@ -753,6 +788,8 @@ static int at24_probe(struct i2c_client >> >> *client, const >> >struct i2c_device_id *id) >> >> goto err_clients; >> > >> >I suppose the runtime PM state is re-initialised for a device when a >> >driver is probed, but it'd still be nice to decrement the use count if this >fails. >> >> Ack >> >> >You should also disable PM runtime if probe fails and set the device >> >suspended again. >> > >> >Same for other error cases. I think you'll need a new label. >> > >> >> Can I disable PM runtime and set suspend in the 'err_clients' label itself ? > >Disable, yes, but the get and put calls need to be balanced. We are performing pm_runtime_put after the first read check and in the error condition, so PM runtime disable alone should be sufficient in the 'err_clients' label. I think it is balanced, your comments ? > >-- >Sakari Ailus >e-mail: sakari.ailus@iki.fi