Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964860AbcKJVAL (ORCPT ); Thu, 10 Nov 2016 16:00:11 -0500 Received: from fllnx210.ext.ti.com ([198.47.19.17]:20085 "EHLO fllnx210.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932302AbcKJVAI (ORCPT ); Thu, 10 Nov 2016 16:00:08 -0500 X-Greylist: delayed 3770 seconds by postgrey-1.27 at vger.kernel.org; Thu, 10 Nov 2016 16:00:08 EST Subject: Re: [PATCH v2 2/4] dt-bindings: Add TI SCI PM Domains To: Rob Herring , Kevin Hilman , Ulf Hansson , Jon Hunter References: <20161019203347.17893-1-d-gerlach@ti.com> <20161019203347.17893-3-d-gerlach@ti.com> <7heg39pmgb.fsf@baylibre.com> <7f4754bb-7ded-6ce3-9558-a45b0d2b3888@ti.com> <7h1sz51y3p.fsf@baylibre.com> <5811FE09.8000006@ti.com> CC: "Rafael J . Wysocki" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-pm@vger.kernel.org" , "devicetree@vger.kernel.org" , Nishanth Menon , Keerthy , Russell King , Tero Kristo , Sudeep Holla , Santosh Shilimkar From: Dave Gerlach Message-ID: <90e91588-d2fc-19be-5a28-63c801a8d061@ti.com> Date: Thu, 10 Nov 2016 13:56:28 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <5811FE09.8000006@ti.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [128.247.83.19] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6780 Lines: 165 Rob, Ulf, Jon, On 10/27/2016 08:15 AM, Dave Gerlach wrote: > +Jon > On 10/26/2016 04:59 PM, Rob Herring wrote: >> On Mon, Oct 24, 2016 at 12:00 PM, Kevin Hilman wrote: >>> Dave Gerlach writes: >>> >>>> Hi, >>>> On 10/21/2016 01:48 PM, Kevin Hilman wrote: >>>>> Dave Gerlach writes: >>>>> >>>>>> Add a generic power domain implementation, TI SCI PM Domains, that >>>>>> will hook into the genpd framework and allow the TI SCI protocol to >>>>>> control device power states. >>>>>> >>>>>> Also, provide macros representing each device index as understood >>>>>> by TI SCI to be used in the device node power-domain references. >>>>>> These are identifiers for the K2G devices managed by the PMMC. >>>>>> >>>>>> Signed-off-by: Nishanth Menon >>>>>> Signed-off-by: Dave Gerlach >>>>>> --- >>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 54 +++++++++++++ >>>>>> MAINTAINERS | 2 + >>>>>> include/dt-bindings/genpd/k2g.h | 90 ++++++++++++++++++++++ >>>>>> 3 files changed, 146 insertions(+) >>>>>> create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>> new file mode 100644 >>>>>> index 000000000000..32f38a349656 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >>>>>> @@ -0,0 +1,54 @@ >>>>>> +Texas Instruments TI-SCI Generic Power Domain >>>>>> +--------------------------------------------- >>>>>> + >>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is >>>>>> +responsible for controlling the state of the IPs that are present. >>>>>> +Communication between the host processor running an OS and the system >>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm domain >>>>>> +implementation plugs into the generic pm domain framework and makes use of >>>>>> +the TI SCI protocol power on and off each device when needed. >>>>>> + >>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt >>>>>> + >>>>>> +PM Domain Node >>>>>> +============== >>>>>> +The PM domain node represents the global PM domain managed by the PMMC, >>>>>> +which in this case is the single implementation as documented by the generic >>>>>> +PM domain bindings in Documentation/devicetree/bindings/power/power_domain.txt. >>>>>> + >>>>>> +Required Properties: >>>>>> +-------------------- >>>>>> +- compatible: should be "ti,sci-pm-domain" >>>>>> +- #power-domain-cells: Must be 0. >>>>>> +- ti,sci: Phandle to the TI SCI device to use for managing the devices. >>>>>> >>>>>> +Example: >>>>>> +-------------------- >>>>>> +k2g_pds: k2g_pds { >>>>> >>>>> should use generic name like "power-contoller", e.g. k2g_pds: power-controller >>>> >>>> Ok, that makes more sense. >>>> >>>>> >>>>>> + compatible = "ti,sci-pm-domain"; >>>>>> + #power-domain-cells = <0>; >>>>>> + ti,sci = <&pmmc>; >>>>>> +}; >>>>>> + >>>>>> +PM Domain Consumers >>>>>> +=================== >>>>>> +Hardware blocks that require SCI control over their state must provide >>>>>> +a reference to the sci-pm-domain they are part of and a unique device >>>>>> +specific ID that identifies the device. >>>>>> + >>>>>> +Required Properties: >>>>>> +-------------------- >>>>>> +- power-domains: phandle pointing to the corresponding PM domain node. >>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to >>>>>> + be used for device control. >>>>> >>>>> This ID doesn't look right. >>>>> >>>>> Why not use #power-domain-cells = <1> and pass the index in the DT? ... >> >> Exactly. ti,sci-id is a NAK for me. > > I was told not to use the onecell during v1 discussion. I agree this would be > ideal but I cannot due to what the bindings represent, the phandle parameter is > an index into a list of genpds, whereas we need an actual ID number we can use > and I do not have the ability to get that from the phandle. > > @Ulf/Jon, is there any hope of bringing back custom xlate functions for genpd > providers? I don't have a good background on why it was even removed. I can > maintain a single genpd for all devices but I need a way to parse this ID, > whether it's from a separate property or a phandle. It is locked now to indexing > into a list of genpds but I need additional per device information for devices > bound to a genpd and I need either a custom parameter or the ability to parse > the phandle myself. > Any comments here? The meaning of the phandle onecell is fixed in the genpd framework so I'm not sure how we want to move forward with this, I need to pass a power domain ID to the genpd driver, and if this shouldn't be a new property I'm not sure what direction we should take. Regards, Dave >> >>>>> >>>>>> +See dt-bindings/genpd/k2g.h for the list of valid identifiers for k2g. >>>>>> + >>>>>> +Example: >>>>>> +-------------------- >>>>>> +uart0: serial@02530c00 { >>>>>> + compatible = "ns16550a"; >>>>>> + ... >>>>>> + power-domains = <&k2g_pds>; >>>>>> + ti,sci-id = ; >>>>> >>>>> ... like this: >>>>> >>>>> power-domains = <&k2g_pds K2G_DEV_UART0>; >>>> >>>> That's how I did it in version one actually. I was able to define my >>>> own xlate function to parse the phandle and get that index, but Ulf >>>> pointed me to this series by Jon Hunter [1] that simplified genpd >>>> providers and dropped the concept of adding your own xlate. This locks >>>> the onecell approach to using a fixed static array of genpds that get >>>> indexed into (without passing the index to the provider, just the >>>> genpd that's looked up), which doesn't fit our usecase, as we don't >>>> want a 1 to 1 genpd to device mapping based on the comments provided >>>> in v1. Now we just use the genpd device attach/detach hooks to parse >>>> the sci-id and then use it in the genpd device start/stop hooks. >> >> I have no idea what any of this means. All sounds like driver >> architecture, not anything to do with bindings. > > This was a response to Kevin, not part of binding description. > >> >>> >>> Ah, right. I remember now. This approach allows you to use a single >>> genpd as discussed earlier. >>> >>> Makes sense now, suggestion retracted. >> >> IIRC, the bindings in Jon's case had a node for each domain and didn't >> need any additional property. > > Yes but we only have one domain and index into it, not into a list of domains, > so the additional property is solving a different problem. > > Regards, > Dave > >> >> Rob >> >