From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grant Likely Date: Tue, 29 Apr 2014 13:16:10 +0000 Subject: Re: [PATCH/RFC 3/4] of/clk: Register clocks suitable for Runtime PM with the PM core Message-Id: <20140429131610.29859C4094A@trevor.secretlab.ca> List-Id: References: <1398334403-26181-1-git-send-email-geert+renesas@glider.be> <1398334403-26181-4-git-send-email-geert+renesas@glider.be> <7hoazpdldx.fsf@paris.lan> In-Reply-To: <7hoazpdldx.fsf@paris.lan> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-arm-kernel@lists.infradead.org On Fri, 25 Apr 2014 16:44:58 -0700, Kevin Hilman wrote: > Geert Uytterhoeven writes: > > > When adding a device from DT, check if its clocks are suitable for Runtime > > PM, and register them with the PM core. > > If Runtime PM is disabled, just enable the clock. > > > > This allows the PM core to automatically manage gate clocks of devices for > > Runtime PM. > > ...unless the device is already in an existing pm_domain, right? > > I like this approach, and it extends nicely what we already do on > platforms using drivers/base/power/clock_ops.c into DT land. > > My only concern is how this will interact if it's used along with > devices that have existing pm_domains. I don't have any specific > concerns (yet, because it's Friday, and my brain is turing off), but it > just made me wonder if this will be potentially confusing. I have big concerns about this approach. First, it will only work if a clock is available at deivce creation time. The conversion of irq controllers to normal device drivers has already shown that is a bad idea. I also don't like that it tries to set up every clock, but there is no guarantee that the driver will even use it. I would rather see this behaviour linked into the function that obtains the clock at driver .probe() time. That way it can handle deferred probe correctly and it only sets up clocks that are actually used by the driver. g. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757477AbaD2NQV (ORCPT ); Tue, 29 Apr 2014 09:16:21 -0400 Received: from mail-ee0-f42.google.com ([74.125.83.42]:62487 "EHLO mail-ee0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757458AbaD2NQQ (ORCPT ); Tue, 29 Apr 2014 09:16:16 -0400 From: Grant Likely Subject: Re: [PATCH/RFC 3/4] of/clk: Register clocks suitable for Runtime PM with the PM core To: Kevin Hilman , Geert Uytterhoeven Cc: Magnus Damm , Simon Horman , Laurent Pinchart , Ben Dooks , Felipe Balbi , Mike Turquette , "Rafael J. Wysocki" , linux-sh@vger.kernel.org, linux-pm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org In-Reply-To: <7hoazpdldx.fsf@paris.lan> References: <1398334403-26181-1-git-send-email-geert+renesas@glider.be> <1398334403-26181-4-git-send-email-geert+renesas@glider.be> <7hoazpdldx.fsf@paris.lan> Date: Tue, 29 Apr 2014 14:16:10 +0100 Message-Id: <20140429131610.29859C4094A@trevor.secretlab.ca> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 25 Apr 2014 16:44:58 -0700, Kevin Hilman wrote: > Geert Uytterhoeven writes: > > > When adding a device from DT, check if its clocks are suitable for Runtime > > PM, and register them with the PM core. > > If Runtime PM is disabled, just enable the clock. > > > > This allows the PM core to automatically manage gate clocks of devices for > > Runtime PM. > > ...unless the device is already in an existing pm_domain, right? > > I like this approach, and it extends nicely what we already do on > platforms using drivers/base/power/clock_ops.c into DT land. > > My only concern is how this will interact if it's used along with > devices that have existing pm_domains. I don't have any specific > concerns (yet, because it's Friday, and my brain is turing off), but it > just made me wonder if this will be potentially confusing. I have big concerns about this approach. First, it will only work if a clock is available at deivce creation time. The conversion of irq controllers to normal device drivers has already shown that is a bad idea. I also don't like that it tries to set up every clock, but there is no guarantee that the driver will even use it. I would rather see this behaviour linked into the function that obtains the clock at driver .probe() time. That way it can handle deferred probe correctly and it only sets up clocks that are actually used by the driver. g. From mboxrd@z Thu Jan 1 00:00:00 1970 From: grant.likely@secretlab.ca (Grant Likely) Date: Tue, 29 Apr 2014 14:16:10 +0100 Subject: [PATCH/RFC 3/4] of/clk: Register clocks suitable for Runtime PM with the PM core In-Reply-To: <7hoazpdldx.fsf@paris.lan> References: <1398334403-26181-1-git-send-email-geert+renesas@glider.be> <1398334403-26181-4-git-send-email-geert+renesas@glider.be> <7hoazpdldx.fsf@paris.lan> Message-ID: <20140429131610.29859C4094A@trevor.secretlab.ca> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, 25 Apr 2014 16:44:58 -0700, Kevin Hilman wrote: > Geert Uytterhoeven writes: > > > When adding a device from DT, check if its clocks are suitable for Runtime > > PM, and register them with the PM core. > > If Runtime PM is disabled, just enable the clock. > > > > This allows the PM core to automatically manage gate clocks of devices for > > Runtime PM. > > ...unless the device is already in an existing pm_domain, right? > > I like this approach, and it extends nicely what we already do on > platforms using drivers/base/power/clock_ops.c into DT land. > > My only concern is how this will interact if it's used along with > devices that have existing pm_domains. I don't have any specific > concerns (yet, because it's Friday, and my brain is turing off), but it > just made me wonder if this will be potentially confusing. I have big concerns about this approach. First, it will only work if a clock is available at deivce creation time. The conversion of irq controllers to normal device drivers has already shown that is a bad idea. I also don't like that it tries to set up every clock, but there is no guarantee that the driver will even use it. I would rather see this behaviour linked into the function that obtains the clock at driver .probe() time. That way it can handle deferred probe correctly and it only sets up clocks that are actually used by the driver. g.