All of lore.kernel.org
 help / color / mirror / Atom feed
From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3] [RFC] arm: use PSCI if available
Date: Wed, 27 Mar 2013 14:10:25 -0500	[thread overview]
Message-ID: <51534421.4050300@gmail.com> (raw)
In-Reply-To: <20130327181206.GD20990@mudshark.cambridge.arm.com>

On 03/27/2013 01:12 PM, Will Deacon wrote:
> On Wed, Mar 27, 2013 at 05:50:51PM +0000, Arnd Bergmann wrote:
>> On Wednesday 27 March 2013, Will Deacon wrote:
>>> The channel is common, sure, but I wouldn't expect the semantics of each
>>> call to be identical between firmware implementations (going back to my
>>> previous examples of CPU IDs and implementation-defined state parameters).
>>>
>>> If a platform happens to have an id-mapping from smp_operations to psci,
>>> then I still think there should be an indirection in there so that we have
>>> the flexibility to change the smp_operations if we wish and not give
>>> platforms the false impression that these two things are equivalent.
>>
>> I think the only reasonably implementation for psci is if we can assume
>> that each callback with a specific property name has a well-defined behavior,
>> and we should mandate that every platform that implements the callbacks
>> we need for SMP actually implements them according to the spec.
>>
>> What would be the point of a standard psci interface if the specific
>> implementation are not required to follow the same semantics?
> 
> The interface *is* standard. The functions have well-defined headers and can
> be called in the same way between implementations. The difference is in the
> semantics of the parameters. For example:
> 
>   int cpu_off(u32 power_state);
> 
> If you look at the power_state parameter, it's actually a struct (see struct
> psci_power_state) with a u16 id field. The current specification describes
> that field as `This is platform specific, the number is understood by the
> firmware, and used to program the power controller.'.
> 
> So unless we get everybody to agree on the definition of that field, we
> can't blindly plug the interfaces together. Furthermore, there are other
> parameters like this and, as new functions are specified, I would expect
> them to grow.

Which is why I've said I think the ID field is a bad idea. I've used it
in my implementation, but only in the case of system level reset,
power-off, and suspend. I don't see how it would be used otherwise.

I guess you could define a value of 0 must be supported at a minimum and
then default implementation would at least work to some level.

Rob

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robherring2@gmail.com>
To: Will Deacon <will.deacon@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	"linux@arm.linux.org.uk" <linux@arm.linux.org.uk>,
	"nico@linaro.org" <nico@linaro.org>,
	Marc Zyngier <Marc.Zyngier@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3] [RFC] arm: use PSCI if available
Date: Wed, 27 Mar 2013 14:10:25 -0500	[thread overview]
Message-ID: <51534421.4050300@gmail.com> (raw)
In-Reply-To: <20130327181206.GD20990@mudshark.cambridge.arm.com>

On 03/27/2013 01:12 PM, Will Deacon wrote:
> On Wed, Mar 27, 2013 at 05:50:51PM +0000, Arnd Bergmann wrote:
>> On Wednesday 27 March 2013, Will Deacon wrote:
>>> The channel is common, sure, but I wouldn't expect the semantics of each
>>> call to be identical between firmware implementations (going back to my
>>> previous examples of CPU IDs and implementation-defined state parameters).
>>>
>>> If a platform happens to have an id-mapping from smp_operations to psci,
>>> then I still think there should be an indirection in there so that we have
>>> the flexibility to change the smp_operations if we wish and not give
>>> platforms the false impression that these two things are equivalent.
>>
>> I think the only reasonably implementation for psci is if we can assume
>> that each callback with a specific property name has a well-defined behavior,
>> and we should mandate that every platform that implements the callbacks
>> we need for SMP actually implements them according to the spec.
>>
>> What would be the point of a standard psci interface if the specific
>> implementation are not required to follow the same semantics?
> 
> The interface *is* standard. The functions have well-defined headers and can
> be called in the same way between implementations. The difference is in the
> semantics of the parameters. For example:
> 
>   int cpu_off(u32 power_state);
> 
> If you look at the power_state parameter, it's actually a struct (see struct
> psci_power_state) with a u16 id field. The current specification describes
> that field as `This is platform specific, the number is understood by the
> firmware, and used to program the power controller.'.
> 
> So unless we get everybody to agree on the definition of that field, we
> can't blindly plug the interfaces together. Furthermore, there are other
> parameters like this and, as new functions are specified, I would expect
> them to grow.

Which is why I've said I think the ID field is a bad idea. I've used it
in my implementation, but only in the case of system level reset,
power-off, and suspend. I don't see how it would be used otherwise.

I guess you could define a value of 0 must be supported at a minimum and
then default implementation would at least work to some level.

Rob

  reply	other threads:[~2013-03-27 19:10 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-27 12:50 [PATCH v3] [RFC] arm: use PSCI if available Stefano Stabellini
2013-03-27 12:50 ` Stefano Stabellini
2013-03-27 12:50 ` Stefano Stabellini
2013-03-27 13:35 ` Marc Zyngier
2013-03-27 13:35   ` Marc Zyngier
2013-03-27 16:20   ` Rob Herring
2013-03-27 16:20     ` Rob Herring
2013-03-27 13:38 ` Will Deacon
2013-03-27 13:38   ` Will Deacon
2013-03-27 16:23   ` Stefano Stabellini
2013-03-27 16:23     ` Stefano Stabellini
2013-03-27 16:35     ` Rob Herring
2013-03-27 16:35       ` Rob Herring
2013-03-27 17:10       ` Stefano Stabellini
2013-03-27 17:10         ` Stefano Stabellini
2013-03-27 17:24         ` Nicolas Pitre
2013-03-27 17:24           ` Nicolas Pitre
2013-03-27 18:22           ` Stefano Stabellini
2013-03-27 18:22             ` Stefano Stabellini
2013-03-27 17:45         ` Rob Herring
2013-03-27 17:45           ` Rob Herring
2013-03-27 18:03           ` Arnd Bergmann
2013-03-27 18:03             ` Arnd Bergmann
2013-03-27 18:14             ` Stefano Stabellini
2013-03-27 18:14               ` Stefano Stabellini
2013-03-27 17:23     ` Will Deacon
2013-03-27 17:23       ` Will Deacon
2013-03-28 12:48       ` Stefano Stabellini
2013-03-28 12:48         ` Stefano Stabellini
2013-03-28 14:51         ` Nicolas Pitre
2013-03-28 14:51           ` Nicolas Pitre
2013-03-28 15:04           ` Rob Herring
2013-03-28 15:04             ` Rob Herring
2013-03-28 15:36             ` Stefano Stabellini
2013-03-28 15:36               ` Stefano Stabellini
2013-03-28 15:39             ` Nicolas Pitre
2013-03-28 15:39               ` Nicolas Pitre
2013-03-28 16:00               ` Will Deacon
2013-03-28 16:00                 ` Will Deacon
2013-03-28 16:06                 ` Nicolas Pitre
2013-03-28 16:06                   ` Nicolas Pitre
2013-03-28 16:20                 ` Stefano Stabellini
2013-03-28 16:20                   ` Stefano Stabellini
2013-03-28 18:38               ` Rob Herring
2013-03-28 18:38                 ` Rob Herring
2013-03-29 13:22                 ` Stefano Stabellini
2013-03-29 13:22                   ` Stefano Stabellini
2013-03-29 13:54                   ` Rob Herring
2013-03-29 13:54                     ` Rob Herring
2013-03-29 14:47                     ` Stefano Stabellini
2013-03-29 14:47                       ` Stefano Stabellini
2013-03-27 16:33   ` Rob Herring
2013-03-27 16:33     ` Rob Herring
2013-03-27 17:05     ` Will Deacon
2013-03-27 17:05       ` Will Deacon
2013-03-27 17:50       ` Arnd Bergmann
2013-03-27 17:50         ` Arnd Bergmann
2013-03-27 18:12         ` Will Deacon
2013-03-27 18:12           ` Will Deacon
2013-03-27 19:10           ` Rob Herring [this message]
2013-03-27 19:10             ` Rob Herring
2013-03-27 19:14           ` Arnd Bergmann
2013-03-27 19:14             ` Arnd Bergmann
2013-03-27 14:55 ` Rob Herring
2013-03-27 14:55   ` Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51534421.4050300@gmail.com \
    --to=robherring2@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.