Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper

From: Felipe Balbi <balbi@ti.com>
To: Vinod Koul <vinod.koul@intel.com>
Cc: linux-wireless@vger.kernel.org, Baruch Siach <baruch@tkos.co.il>,
	patches@opensource.wolfsonmicro.com, linux-doc@vger.kernel.org,
	linux-mmc@vger.kernel.org, Daniel Vetter <daniel.vetter@ffwll.ch>,
	Chris Ball <chris@printf.net>,
	dri-devel@lists.freedesktop.org, Marcin Wojtas <mw@semihalf.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	David Herrmann <dh.herrmann@gmail.com>,
	Shinya Kuribayashi <skuribay@pobox.com>,
	Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
	Jingoo Han <jg1.han@samsung.com>,
	Seungwon Jeon <tgih.jun@samsung.com>,
	Jaehoon Chung <jh80.chung@samsung.com>,
	Jani Nikula <jani.nikula@intel.com>,
	Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	Ben Skeggs <bskeggs@redhat.com>,
	Bjorn Andersson <bjorn.andersson@sonymobile.com>,
	Wolfram Sang <wsa@the-dreams
Subject: Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
Date: Thu, 25 Sep 2014 09:22:01 -0500	[thread overview]
Message-ID: <20140925142201.GD28045@saruman> (raw)
In-Reply-To: <20140925075718.GW24663@intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 1737 bytes --]

On Thu, Sep 25, 2014 at 01:27:18PM +0530, Vinod Koul wrote:
> On Wed, Sep 24, 2014 at 03:32:19PM -0500, Felipe Balbi wrote:
> > > > > OK, I guess this is as good as it gets.
> > > > > 
> > > > > What tree would you like it go through?
> > > > 
> > > > Do we really need this new helper ? I mean, the very moment when we
> > > > decide to implement ->runtime_idle() we will need to get rid of this
> > > > change. I wonder if it's really valid...
> > > 
> > > I'm not sure I'm following?  This seems to simply implement what drivers
> > > have been doing already as one function.  Why would it be invalid to reduce
> > > code duplication?
> > 
> > For two reasons:
> > 
> > 1) the helper has no inteligence whatsoever. It just calls the same
> > functions.
> > 
> > 2) the duplication will vanish whenever someone implements
> > ->runtime_idle() and have that call pm_runtime_autosuspend() (like PCI
> > and USB buses are doing today). This will just be yet another line that
> > needs to change.
> > 
> > Frankly though, no strong feelings, I just think it's a commit that
> > doesn't bring that any benefits other than looking like one line was
> > removed.
> and yes that is what it tries to do nothing more nothing less. If in future
> there are no users (today we have quite a few), then we can remove the dead
> macro, no harm. But that is not the situation today.

as I said, a commit that's bound to be useless. It's not like you're
saving 10 lines of code, it's only one. Replacing two simple lines with
a function which takes <joke> almost as many characters to type </joke>.

IMO, this is pretty useless and I'd rather not see them in the drivers I
maintain, sorry.

cheers

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx