Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

From: Ulf Hansson <ulf.hansson@linaro.org>
To: Rob Herring <robh@kernel.org>, Kevin Hilman <khilman@baylibre.com>
Cc: Tero Kristo <t-kristo@ti.com>, Dave Gerlach <d-gerlach@ti.com>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Nishanth Menon <nm@ti.com>, Keerthy <j-keerthy@ti.com>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	Lokesh Vutla <lokeshvutla@ti.com>
Subject: Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains
Date: Fri, 20 Jan 2017 15:00:02 +0100	[thread overview]
Message-ID: <CAPDyKFqzs59N5bc89X-96Fgy9b=_+s8zYmHHshQeDrJCnaqprA@mail.gmail.com> (raw)
In-Reply-To: <CAL_Jsq+Vp8iYrAUmuSRzeeY_G6CUqFqG6XB6erDxo1RYo3g4=g@mail.gmail.com>

+ Sudeep

On 19 January 2017 at 00:03, Rob Herring <robh@kernel.org> wrote:
> On Tue, Jan 17, 2017 at 6:07 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>> Tero Kristo <t-kristo@ti.com> writes:
>>> On 17/01/17 00:12, Dave Gerlach wrote:
>>>> On 01/13/2017 08:40 PM, Rob Herring wrote:
>>>>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>
> [...]
>
>>>>>> My ti,sci-id is not an index into a list of power domains, so it
>>>>>> should not
>>>>>> go in the power-domains cells and go against what the power-domains
>>>>>> binding
>>>>>> says that the cell expects. We have one single power domain, and the new
>>>>>> ti,sci-id binding is not something the genpd framework itself is
>>>>>> concerned
>>>>>> with as it's our property to identify a device inside a power domain,
>>>>>> not to
>>>>>> identify which power domain it is associated with.
>>>>>
>>>>> What is the id used for? I can understand why you need to know what
>>>>> power domain a device is in (as power-domains identifies), but not
>>>>> what devices are in a power domain.
>>>>
>>>> We have a system control processor that provides power management
>>>> services to the OS and it responsible for handling the power state of
>>>> each device. This control happens over a communication interface we have
>>>> called TI SCI (implemented at drivers/firmware/ti-sci.c). The
>>>> communication protocol uses these ids to identify each device within the
>>>> power domain so that the control processor can do what is necessary to
>>>> enable that device.
>>>
>>> I think a minor detail here that Rob might be missing right now is,
>>> that the ti,sci-id is only controlling the PM runtime handling, and
>>> providing the ID per-device for this purpose only. AFAIK, it is not
>>> really connected to the power domain anymore as such, as we don't have
>>> power-domains / per device anymore as was the case in some earlier
>>> revision of this work.
>>
>> I think this gets to the heart of things.  IMO The confusion arises
>> because we're throwing around the term "power domain" when there isn't
>> an actual hardware power domain here.
>
> I thought there was 1.
>
>> Unfortunately, the genpd bindings have used the terminology power-domain
>> when in fact genpd is more generic than that and can be used not just
>> for hardware power domains, but for arbitrary grouping of devices that
>> have common PM properties.  That's why genpd actually stands for generic
>> PM domain, not power domain.  Unfortunately, the bindings have grown
>> primarily out of the usage for hardware power domains.
>
> Now it makes some sense.
>
> So the question is does this PM domain grouping need to be described
> in DT or not, and if so what does that look like?

Yes, it's needed and already being done. For example, we have clock
domains for several Renesas platforms.

>
> We could continue to use the power domain binding (maybe we already
> are and that ship has sailed). I'm not totally against the idea even
> if there is no power domain, but I'm not sold on it either. If we do
> go this route, then I still say the id should be a cell in the
> power-domain phandle.
>
> Another option is create something new either common or TI SCI
> specific. It could be just a table of ids and phandles in the SCI
> node. I'm much more comfortable with an isolated property in one node
> than something scattered throughout the DT.

To me, this seems like the best possible solution.

However, perhaps we should also consider the SCPI Generic power domain
(drivers/firmware/scpi_pm_domain.c), because I believe it's closely
related.
To change the power state of a device, this PM domain calls
scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
also needs a device id as a parameter. Very similar to our case with
the TI SCI domain.

Currently these SCPI device ids lacks corresponding DT bindings, so
the scpi_pm_domain tries to work around it by assigning ids
dynamically at genpd creation time.

That makes me wonder, whether we should think of something common/generic?

[...]

Kind regards
Uffe