Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755152AbaKRSzZ (ORCPT ); Tue, 18 Nov 2014 13:55:25 -0500 Received: from devils.ext.ti.com ([198.47.26.153]:55866 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755097AbaKRSzW (ORCPT ); Tue, 18 Nov 2014 13:55:22 -0500 Message-ID: <546B95EC.5000705@ti.com> Date: Tue, 18 Nov 2014 20:54:36 +0200 From: Grygorii Strashko User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Kevin Hilman , Arnd Bergmann CC: , "Rafael J. Wysocki" , , Rob Herring , , , , , Ulf Hansson , Geert Uytterhoeven , Dmitry Torokhov Subject: Re: [PATCH v4 1/2] ARM: keystone: pm: switch to use generic pm domains References: <1415631557-22897-1-git-send-email-grygorii.strashko@ti.com> <1600093.60yAP0Qpua@wuerfel> <7h389h3aif.fsf@deeprootsystems.com> <1618855.DXsjWXLqau@wuerfel> <7hsihhzecq.fsf@deeprootsystems.com> In-Reply-To: <7hsihhzecq.fsf@deeprootsystems.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi All, Thank you for your comments. On 11/17/2014 11:50 PM, Kevin Hilman wrote: > Arnd Bergmann writes: > >> On Monday 17 November 2014 11:14:16 Kevin Hilman wrote: >>>>> >>>>> So, The Keystone 2 Generic PM Controller is just a proxy PM layer here between >>>>> device and Generic clock manipulation PM callbacks. >>>>> It fills per-device clock list when device is attached to GPD and >>>>> ensures that all clocks from that list enabled/disabled when device is >>>>> started/stopped. >>>> >>>> The idea of such a generic power domain implementation sounds useful, but >>>> it has absolutely no business in platform specific code. >>> >>> Yes it does. This isn't a generic power domain implementation, but >>> rather just the platform-specific glue that hooks up the clocks to the >>> right devices and power-domains so that the generic power-domain and >>> generic pm_clocks code does the right thing. >> >> How would you do this on an arm64 version of keystone then? With >> the current approach, you'd need to add a machine specific directory, >> and that seems completely pointless since this is not even about >> a hardware requirement. > > Yeah, you're right. I misunderstood you're original comment. > >>>> I suggest you either remove the power domain proxy from your drivers >>>> and use the clocks directly, Hm. I've been thinking about this, but the problem is that Keystone 2 reuses a lot of IPs from Davinci and PM for Davinci is based on Generic clock manipulation PM callbacks framework, but for non-DT case. So, I can't simply use clocks directly. >>> >>> No. That's a step in the wrong direction. This change isn't affecting >>> drivers directly. It's the runtime PM and generic power domain layers >>> that handle this, and runtime PM adapted drivers don't need any changes. >>> >>>> or come up with an implementation that can be used across other >>>> platforms and CPU architectures. >>> >>> We already have those in the generic power domain and the pm_clock >>> layers. This series is just hooking those up for Keystone. >> >> Then why not add the missing piece to the generic power domain >> code to avoid having to add infrastructure to the platform >> for it? > > Yes, good point. There is nothing keystone-specific in this glue. > > Grygorii, what about adding a feature to the generic domain parsing so > that it can get clocks from device nodes that are part of the domain, > and so it sets up pm_clk accordingly. I'd like to mention few points here: 1) not all platforms may need this 2) not all platforms may allow to add ALL clocks from "clocks" property to pm_clk as some of them can be optional or have to be controlled by drivers only (for example, initially, it was the case for SH-mobile https://lkml.org/lkml/2014/4/24/197 also now, last implementation for shmobile add only first clock from "clocks" property to pm_clk https://lkml.org/lkml/2014/11/17/272). 3) such functionality have to be enabled for devices selectively, for example now we are going to enable it for devices which a ready for runtime PM. Current implementation cover 1 & 3, but also it allows to cover 2 too, because it's platform specific implementation and .attach_dev() can be updated to skip some clocks or devices if needed. > > I've recently seen other SoCs doing very similar, so this really should > be generalized. > > I've been looking at this primarily as a right incremental improvement > from what is there for Keystone today, but Arnd is right. This should > be moved out of platform code. I'm ready to do what ever you want, but I don't fully understand what exactly to do :( Should I create some generic_pm_clk_domain.c? - or - Do you mean to integrate it in domain.c (see no way to do it:()? - or - smth. else What about introduced DT bindings? For example, How will devices be selected for attachment to Generic pm_clk domain if I'll introduce generic_pm_clk_domain.c? Regards, -grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/