* [PATCH 02/16] ARM: b.L: introduce the CPU/cluster power API
From: Santosh Shilimkar @ 2013-01-11 17:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-3-git-send-email-nicolas.pitre@linaro.org>
On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
> This is the basic API used to handle the powering up/down of individual
> CPUs in a big.LITTLE system. The platform specific backend implementation
> has the responsibility to also handle the cluster level power as well when
> the first/last CPU in a cluster is brought up/down.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/common/bL_entry.c | 88 +++++++++++++++++++++++++++++++++++++++
> arch/arm/include/asm/bL_entry.h | 92 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 180 insertions(+)
>
> diff --git a/arch/arm/common/bL_entry.c b/arch/arm/common/bL_entry.c
> index 80fff49417..41de0622de 100644
> --- a/arch/arm/common/bL_entry.c
> +++ b/arch/arm/common/bL_entry.c
> @@ -11,11 +11,13 @@
>
> #include <linux/kernel.h>
> #include <linux/init.h>
> +#include <linux/irqflags.h>
>
> #include <asm/bL_entry.h>
> #include <asm/barrier.h>
> #include <asm/proc-fns.h>
> #include <asm/cacheflush.h>
> +#include <asm/idmap.h>
>
> extern volatile unsigned long bL_entry_vectors[BL_NR_CLUSTERS][BL_CPUS_PER_CLUSTER];
>
> @@ -28,3 +30,89 @@ void bL_set_entry_vector(unsigned cpu, unsigned cluster, void *ptr)
> outer_clean_range(__pa(&bL_entry_vectors[cluster][cpu]),
> __pa(&bL_entry_vectors[cluster][cpu + 1]));
> }
> +
> +static const struct bL_platform_power_ops *platform_ops;
> +
> +int __init bL_platform_power_register(const struct bL_platform_power_ops *ops)
> +{
> + if (platform_ops)
> + return -EBUSY;
> + platform_ops = ops;
> + return 0;
> +}
> +
> +int bL_cpu_power_up(unsigned int cpu, unsigned int cluster)
> +{
> + if (!platform_ops)
> + return -EUNATCH;
> + might_sleep();
> + return platform_ops->power_up(cpu, cluster);
> +}
> +
> +typedef void (*phys_reset_t)(unsigned long);
> +
> +void bL_cpu_power_down(void)
> +{
> + phys_reset_t phys_reset;
> +
> + BUG_ON(!platform_ops);
> + BUG_ON(!irqs_disabled());
> +
> + /*
> + * Do this before calling into the power_down method,
> + * as it might not always be safe to do afterwards.
> + */
> + setup_mm_for_reboot();
> +
> + platform_ops->power_down();
> +
> + /*
> + * It is possible for a power_up request to happen concurrently
> + * with a power_down request for the same CPU. In this case the
> + * power_down method might not be able to actually enter a
> + * powered down state with the WFI instruction if the power_up
> + * method has removed the required reset condition. The
> + * power_down method is then allowed to return. We must perform
> + * a re-entry in the kernel as if the power_up method just had
> + * deasserted reset on the CPU.
> + *
> + * To simplify race issues, the platform specific implementation
> + * must accommodate for the possibility of unordered calls to
> + * power_down and power_up with a usage count. Therefore, if a
> + * call to power_up is issued for a CPU that is not down, then
> + * the next call to power_down must not attempt a full shutdown
> + * but only do the minimum (normally disabling L1 cache and CPU
> + * coherency) and return just as if a concurrent power_up request
> + * had happened as described above.
> + */
> +
> + phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
> + phys_reset(virt_to_phys(bL_entry_point));
> +
> + /* should never get here */
> + BUG();
> +}
> +
> +void bL_cpu_suspend(u64 expected_residency)
> +{
> + phys_reset_t phys_reset;
> +
> + BUG_ON(!platform_ops);
> + BUG_ON(!irqs_disabled());
> +
> + /* Very similar to bL_cpu_power_down() */
> + setup_mm_for_reboot();
> + platform_ops->suspend(expected_residency);
> + phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
> + phys_reset(virt_to_phys(bL_entry_point));
> + BUG();
>
I might be missing all the rationales behind not having a recovery for
CPUs entering suspend if they actualy come here because of some events.
This is pretty much possible in many scenario's and hence letting CPU
cpu come out of suspend should be possible. May be switcher code don't
have such requirement but it appeared bit off to me.
Regards
Santosh
^ permalink raw reply
* [kvmarm] [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Christoffer Dall @ 2013-01-11 17:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F04AD5.6020703@arm.com>
On Fri, Jan 11, 2013 at 12:24 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 11/01/13 17:12, Russell King - ARM Linux wrote:
>> On Thu, Jan 10, 2013 at 04:06:45PM +0000, Marc Zyngier wrote:
>>> +int kvm_psci_call(struct kvm_vcpu *vcpu)
>>> +{
>>> + unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>> + unsigned long val;
>>> +
>>> + switch (psci_fn) {
>>> + case KVM_PSCI_FN_CPU_OFF:
>>> + kvm_psci_vcpu_off(vcpu);
>>> + val = KVM_PSCI_RET_SUCCESS;
>>> + break;
>>> + case KVM_PSCI_FN_CPU_ON:
>>> + val = kvm_psci_vcpu_on(vcpu);
>>> + break;
>>> + case KVM_PSCI_FN_CPU_SUSPEND:
>>> + case KVM_PSCI_FN_MIGRATE:
>>> + val = KVM_PSCI_RET_NI;
>>> + break;
>>> +
>>> + default:
>>> + return -1;
>>> + }
>>> +
>>> + *vcpu_reg(vcpu, 0) = val;
>>> + return 0;
>>> +}
>>
>> We were discussing recently on #kernel about kernel APIs and the way that
>> our integer-returning functions pretty much use 0 for success, and -errno
>> for failures, whereas our pointer-returning functions are a mess.
>>
>> And above we have something returning -1 to some other chunk of code outside
>> this compilation unit. That doesn't sound particularly clever to me.
>
> The original code used to return -EINVAL, see:
> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-January/004509.html
>
> Christoffer (Cc-ed) didn't like this, hence the -1. I'm happy to revert
> the code to its original state though.
>
I don't want to return -EINVAL, because for the rest of the KVM code
this would mean kill the guest.
The convention used in other archs of KVM as well as for ARM is that
the handle_exit functions return:
-ERRNO: Error, report this error to user space
0: Everything is fine, but return to user space to let it do I/O
emulation and whatever it wants to do
1: Everything is fine, return directly to the guest without going to user space
And then you do:
if (handle_something() == 0)
return 1;
which I thought was confusing, so I said make the function a bool, to
avoid the confusion, like Rusty did for all the coprocessor emulation
functions.
There are obviously other ways to handle the "return 1" case, like
having an extra state that you carry around, and we can change all the
code to do that, but I just don't think it's worth it, as we are in
fact quite close to the existing kernel API.
-Christoffer
^ permalink raw reply
* [PATCH 0/3] arch_decomp_wdog cleanup
From: Kukjin Kim @ 2013-01-11 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111170843.GG23505@n2100.arm.linux.org.uk>
Russell King - ARM Linux wrote:
>
> On Thu, Jan 10, 2013 at 10:12:42PM +0800, Shawn Guo wrote:
> > All the arch_decomp_wdog related codes are there without any users,
> > since ARCH_HAS_DECOMP_WDOG is only used by lib/inflate.c which
> however
> > is not used by arch/arm/boot/compressed/decompress.c.
>
> Hmm, this suggests that those samsung platforms haven't been booted for
> a while - because the decompressor will enable the watchdog, but won't
> pat it at all. So, do we still need them in the kernel?
>
Hmm, I didn't see its usage lately on relatively Samsun SoCs. But let me
check again, the code is not used on every Samsung SoCs. So please hold on a
couple of days, I already reply 'ack' on Samsung stuff though.
Thanks.
- Kukjin
^ permalink raw reply
* [kvmarm] [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Russell King - ARM Linux @ 2013-01-11 17:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANM98qJ2ZSwxMWFRWiunTQtoV7iFb3fn3Cf6VWvOndBcU=+Tmg@mail.gmail.com>
On Fri, Jan 11, 2013 at 12:33:15PM -0500, Christoffer Dall wrote:
> On Fri, Jan 11, 2013 at 12:24 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > On 11/01/13 17:12, Russell King - ARM Linux wrote:
> >> On Thu, Jan 10, 2013 at 04:06:45PM +0000, Marc Zyngier wrote:
> >>> +int kvm_psci_call(struct kvm_vcpu *vcpu)
> >>> +{
> >>> + unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
> >>> + unsigned long val;
> >>> +
> >>> + switch (psci_fn) {
> >>> + case KVM_PSCI_FN_CPU_OFF:
> >>> + kvm_psci_vcpu_off(vcpu);
> >>> + val = KVM_PSCI_RET_SUCCESS;
> >>> + break;
> >>> + case KVM_PSCI_FN_CPU_ON:
> >>> + val = kvm_psci_vcpu_on(vcpu);
> >>> + break;
> >>> + case KVM_PSCI_FN_CPU_SUSPEND:
> >>> + case KVM_PSCI_FN_MIGRATE:
> >>> + val = KVM_PSCI_RET_NI;
> >>> + break;
> >>> +
> >>> + default:
> >>> + return -1;
> >>> + }
> >>> +
> >>> + *vcpu_reg(vcpu, 0) = val;
> >>> + return 0;
> >>> +}
> >>
> >> We were discussing recently on #kernel about kernel APIs and the way that
> >> our integer-returning functions pretty much use 0 for success, and -errno
> >> for failures, whereas our pointer-returning functions are a mess.
> >>
> >> And above we have something returning -1 to some other chunk of code outside
> >> this compilation unit. That doesn't sound particularly clever to me.
> >
> > The original code used to return -EINVAL, see:
> > https://lists.cs.columbia.edu/pipermail/kvmarm/2013-January/004509.html
> >
> > Christoffer (Cc-ed) didn't like this, hence the -1. I'm happy to revert
> > the code to its original state though.
> >
> I don't want to return -EINVAL, because for the rest of the KVM code
> this would mean kill the guest.
>
> The convention used in other archs of KVM as well as for ARM is that
> the handle_exit functions return:
>
> -ERRNO: Error, report this error to user space
> 0: Everything is fine, but return to user space to let it do I/O
> emulation and whatever it wants to do
> 1: Everything is fine, return directly to the guest without going to user space
Right, so the above "return -1" _is_ doing the thing that I really hate,
which is it's actually doing a "return -EPERM" without anyone realising
that's what it's doing.
This is precisely why I hate (and pick up on, and have my mail reader to
highlight) any "return -1". It's mostly always a bug.
^ permalink raw reply
* [kvmarm] [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Marc Zyngier @ 2013-01-11 17:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANM98qJ2ZSwxMWFRWiunTQtoV7iFb3fn3Cf6VWvOndBcU=+Tmg@mail.gmail.com>
On 11/01/13 17:33, Christoffer Dall wrote:
> On Fri, Jan 11, 2013 at 12:24 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 11/01/13 17:12, Russell King - ARM Linux wrote:
>>> On Thu, Jan 10, 2013 at 04:06:45PM +0000, Marc Zyngier wrote:
>>>> +int kvm_psci_call(struct kvm_vcpu *vcpu)
>>>> +{
>>>> + unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>>> + unsigned long val;
>>>> +
>>>> + switch (psci_fn) {
>>>> + case KVM_PSCI_FN_CPU_OFF:
>>>> + kvm_psci_vcpu_off(vcpu);
>>>> + val = KVM_PSCI_RET_SUCCESS;
>>>> + break;
>>>> + case KVM_PSCI_FN_CPU_ON:
>>>> + val = kvm_psci_vcpu_on(vcpu);
>>>> + break;
>>>> + case KVM_PSCI_FN_CPU_SUSPEND:
>>>> + case KVM_PSCI_FN_MIGRATE:
>>>> + val = KVM_PSCI_RET_NI;
>>>> + break;
>>>> +
>>>> + default:
>>>> + return -1;
>>>> + }
>>>> +
>>>> + *vcpu_reg(vcpu, 0) = val;
>>>> + return 0;
>>>> +}
>>>
>>> We were discussing recently on #kernel about kernel APIs and the way that
>>> our integer-returning functions pretty much use 0 for success, and -errno
>>> for failures, whereas our pointer-returning functions are a mess.
>>>
>>> And above we have something returning -1 to some other chunk of code outside
>>> this compilation unit. That doesn't sound particularly clever to me.
>>
>> The original code used to return -EINVAL, see:
>> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-January/004509.html
>>
>> Christoffer (Cc-ed) didn't like this, hence the -1. I'm happy to revert
>> the code to its original state though.
>>
> I don't want to return -EINVAL, because for the rest of the KVM code
> this would mean kill the guest.
>
> The convention used in other archs of KVM as well as for ARM is that
> the handle_exit functions return:
>
> -ERRNO: Error, report this error to user space
> 0: Everything is fine, but return to user space to let it do I/O
> emulation and whatever it wants to do
> 1: Everything is fine, return directly to the guest without going to user space
That is assuming we propagate the handle_exit convention down to the
leaf calls, and I object to that. The 3 possible values only apply to
handle_exit, and we should keep that convention as local as possible,
because this is the odd case.
> And then you do:
> if (handle_something() == 0)
> return 1;
>
> which I thought was confusing, so I said make the function a bool, to
> avoid the confusion, like Rusty did for all the coprocessor emulation
> functions.
I don't see a compelling reason to propagate this convention to areas
that do not require it. In the PSCI case, we have a basic
handled/not-handled state, the later indicating the reason. The exit
handling functions can convert the error codes to whatever the run loop
requires.
> There are obviously other ways to handle the "return 1" case, like
> having an extra state that you carry around, and we can change all the
> code to do that, but I just don't think it's worth it, as we are in
> fact quite close to the existing kernel API.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH 03/16] ARM: b.L: introduce helpers for platform coherency exit/setup
From: Santosh Shilimkar @ 2013-01-11 17:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-4-git-send-email-nicolas.pitre@linaro.org>
On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
> From: Dave Martin <dave.martin@linaro.org>
>
> This provides helper methods to coordinate between CPUs coming down
> and CPUs going up, as well as documentation on the used algorithms,
> so that cluster teardown and setup
> operations are not done for a cluster simultaneously.
>
> For use in the power_down() implementation:
> * __bL_cpu_going_down(unsigned int cluster, unsigned int cpu)
> * __bL_outbound_enter_critical(unsigned int cluster)
> * __bL_outbound_leave_critical(unsigned int cluster)
> * __bL_cpu_down(unsigned int cluster, unsigned int cpu)
>
> The power_up_setup() helper should do platform-specific setup in
> preparation for turning the CPU on, such as invalidating local caches
> or entering coherency. It must be assembler for now, since it must
> run before the MMU can be switched on. It is passed the affinity level
> which should be initialized.
>
> Because the bL_cluster_sync_struct content is looked-up and modified
> with the cache enabled or disabled depending on the code path, it is
> crucial to always ensure proper cache maintenance to update main memory
> right away. Therefore, any cached write must be followed by a cache clean
> operation and any cached read must be preceded by a cache invalidate
> operation on the accessed memory.
>
> To avoid races where a reader would invalidate the cache and discard the
> latest update from a writer before that writer had a chance to clean it
> to RAM, we simply use cache flush (clean+invalidate) operations
> everywhere.
>
> Also, in order to prevent a cached writer from interfering with an
> adjacent non-cached writer, we ensure each state variable is located to
> a separate cache line.
>
> Thanks to Nicolas Pitre and Achin Gupta for the help with this
> patch.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
> .../arm/big.LITTLE/cluster-pm-race-avoidance.txt | 498 +++++++++++++++++++++
> arch/arm/common/bL_entry.c | 160 +++++++
> arch/arm/common/bL_head.S | 88 +++-
> arch/arm/include/asm/bL_entry.h | 62 +++
> 4 files changed, 806 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt
>
> diff --git a/Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt b/Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt
> new file mode 100644
> index 0000000000..d6151e0235
> --- /dev/null
> +++ b/Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt
> @@ -0,0 +1,498 @@
> +Big.LITTLE cluster Power-up/power-down race avoidance algorithm
> +===============================================================
> +
> +This file documents the algorithm which is used to coordinate CPU and
> +cluster setup and teardown operations and to manage hardware coherency
> +controls safely.
> +
> +The section "Rationale" explains what the algorithm is for and why it is
> +needed. "Basic model" explains general concepts using a simplified view
> +of the system. The other sections explain the actual details of the
> +algorithm in use.
> +
> +
> +Rationale
> +---------
> +
> +In a system containing multiple CPUs, it is desirable to have the
> +ability to turn off individual CPUs when the system is idle, reducing
> +power consumption and thermal dissipation.
> +
> +In a system containing multiple clusters of CPUs, it is also desirable
> +to have the ability to turn off entire clusters.
> +
> +Turning entire clusters off and on is a risky business, because it
> +involves performing potentially destructive operations affecting a group
> +of independently running CPUs, while the OS continues to run. This
> +means that we need some coordination in order to ensure that critical
> +cluster-level operations are only performed when it is truly safe to do
> +so.
> +
> +Simple locking may not be sufficient to solve this problem, because
> +mechanisms like Linux spinlocks may rely on coherency mechanisms which
> +are not immediately enabled when a cluster powers up. Since enabling or
> +disabling those mechanisms may itself be a non-atomic operation (such as
> +writing some hardware registers and invalidating large caches), other
> +methods of coordination are required in order to guarantee safe
> +power-down and power-up at the cluster level.
> +
> +The mechanism presented in this document describes a coherent memory
> +based protocol for performing the needed coordination. It aims to be as
> +lightweight as possible, while providing the required safety properties.
> +
> +
> +Basic model
> +-----------
> +
> +Each cluster and CPU is assigned a state, as follows:
> +
> + DOWN
> + COMING_UP
> + UP
> + GOING_DOWN
> +
> + +---------> UP ----------+
> + | v
> +
> + COMING_UP GOING_DOWN
> +
> + ^ |
> + +--------- DOWN <--------+
> +
> +
> +DOWN: The CPU or cluster is not coherent, and is either powered off or
> + suspended, or is ready to be powered off or suspended.
> +
> +COMING_UP: The CPU or cluster has committed to moving to the UP state.
> + It may be part way through the process of initialisation and
> + enabling coherency.
> +
> +UP: The CPU or cluster is active and coherent at the hardware
> + level. A CPU in this state is not necessarily being used
> + actively by the kernel.
> +
> +GOING_DOWN: The CPU or cluster has committed to moving to the DOWN
> + state. It may be part way through the process of teardown and
> + coherency exit.
> +
> +
> +Each CPU has one of these states assigned to it at any point in time.
> +The CPU states are described in the "CPU state" section, below.
> +
> +Each cluster is also assigned a state, but it is necessary to split the
> +state value into two parts (the "cluster" state and "inbound" state) and
> +to introduce additional states in order to avoid races between different
> +CPUs in the cluster simultaneously modifying the state. The cluster-
> +level states are described in the "Cluster state" section.
> +
> +To help distinguish the CPU states from cluster states in this
> +discussion, the state names are given a CPU_ prefix for the CPU states,
> +and a CLUSTER_ or INBOUND_ prefix for the cluster states.
> +
> +
> +CPU state
> +---------
> +
> +In this algorithm, each individual core in a multi-core processor is
> +referred to as a "CPU". CPUs are assumed to be single-threaded:
> +therefore, a CPU can only be doing one thing at a single point in time.
> +
> +This means that CPUs fit the basic model closely.
> +
> +The algorithm defines the following states for each CPU in the system:
> +
> + CPU_DOWN
> + CPU_COMING_UP
> + CPU_UP
> + CPU_GOING_DOWN
> +
> + cluster setup and
> + CPU setup complete policy decision
> + +-----------> CPU_UP ------------+
> + | v
> +
> + CPU_COMING_UP CPU_GOING_DOWN
> +
> + ^ |
> + +----------- CPU_DOWN <----------+
> + policy decision CPU teardown complete
> + or hardware event
> +
> +
> +The definitions of the four states correspond closely to the states of
> +the basic model.
> +
> +Transitions between states occur as follows.
> +
> +A trigger event (spontaneous) means that the CPU can transition to the
> +next state as a result of making local progress only, with no
> +requirement for any external event to happen.
> +
> +
> +CPU_DOWN:
> +
> + A CPU reaches the CPU_DOWN state when it is ready for
> + power-down. On reaching this state, the CPU will typically
> + power itself down or suspend itself, via a WFI instruction or a
> + firmware call.
> +
> + Next state: CPU_COMING_UP
> + Conditions: none
> +
> + Trigger events:
> +
> + a) an explicit hardware power-up operation, resulting
> + from a policy decision on another CPU;
> +
> + b) a hardware event, such as an interrupt.
> +
> +
> +CPU_COMING_UP:
> +
> + A CPU cannot start participating in hardware coherency until the
> + cluster is set up and coherent. If the cluster is not ready,
> + then the CPU will wait in the CPU_COMING_UP state until the
> + cluster has been set up.
> +
> + Next state: CPU_UP
> + Conditions: The CPU's parent cluster must be in CLUSTER_UP.
> + Trigger events: Transition of the parent cluster to CLUSTER_UP.
> +
> + Refer to the "Cluster state" section for a description of the
> + CLUSTER_UP state.
> +
> +
> +CPU_UP:
> + When a CPU reaches the CPU_UP state, it is safe for the CPU to
> + start participating in local coherency.
> +
> + This is done by jumping to the kernel's CPU resume code.
> +
> + Note that the definition of this state is slightly different
> + from the basic model definition: CPU_UP does not mean that the
> + CPU is coherent yet, but it does mean that it is safe to resume
> + the kernel. The kernel handles the rest of the resume
> + procedure, so the remaining steps are not visible as part of the
> + race avoidance algorithm.
> +
> + The CPU remains in this state until an explicit policy decision
> + is made to shut down or suspend the CPU.
> +
> + Next state: CPU_GOING_DOWN
> + Conditions: none
> + Trigger events: explicit policy decision
> +
> +
> +CPU_GOING_DOWN:
> +
> + While in this state, the CPU exits coherency, including any
> + operations required to achieve this (such as cleaning data
> + caches).
> +
> + Next state: CPU_DOWN
> + Conditions: local CPU teardown complete
> + Trigger events: (spontaneous)
> +
> +
> +Cluster state
> +-------------
> +
> +A cluster is a group of connected CPUs with some common resources.
> +Because a cluster contains multiple CPUs, it can be doing multiple
> +things at the same time. This has some implications. In particular, a
> +CPU can start up while another CPU is tearing the cluster down.
> +
> +In this discussion, the "outbound side" is the view of the cluster state
> +as seen by a CPU tearing the cluster down. The "inbound side" is the
> +view of the cluster state as seen by a CPU setting the CPU up.
> +
> +In order to enable safe coordination in such situations, it is important
> +that a CPU which is setting up the cluster can advertise its state
> +independently of the CPU which is tearing down the cluster. For this
> +reason, the cluster state is split into two parts:
> +
> + "cluster" state: The global state of the cluster; or the state
> + on the outbound side:
> +
> + CLUSTER_DOWN
> + CLUSTER_UP
> + CLUSTER_GOING_DOWN
> +
> + "inbound" state: The state of the cluster on the inbound side.
> +
> + INBOUND_NOT_COMING_UP
> + INBOUND_COMING_UP
> +
> +
> + The different pairings of these states results in six possible
> + states for the cluster as a whole:
> +
> + CLUSTER_UP
> + +==========> INBOUND_NOT_COMING_UP -------------+
> + # |
> + |
> + CLUSTER_UP <----+ |
> + INBOUND_COMING_UP | v
> +
> + ^ CLUSTER_GOING_DOWN CLUSTER_GOING_DOWN
> + # INBOUND_COMING_UP <=== INBOUND_NOT_COMING_UP
> +
> + CLUSTER_DOWN | |
> + INBOUND_COMING_UP <----+ |
> + |
> + ^ |
> + +=========== CLUSTER_DOWN <------------+
> + INBOUND_NOT_COMING_UP
> +
> + Transitions -----> can only be made by the outbound CPU, and
> + only involve changes to the "cluster" state.
> +
> + Transitions ===##> can only be made by the inbound CPU, and only
> + involve changes to the "inbound" state, except where there is no
> + further transition possible on the outbound side (i.e., the
> + outbound CPU has put the cluster into the CLUSTER_DOWN state).
> +
> + The race avoidance algorithm does not provide a way to determine
> + which exact CPUs within the cluster play these roles. This must
> + be decided in advance by some other means. Refer to the section
> + "Last man and first man selection" for more explanation.
> +
> +
> + CLUSTER_DOWN/INBOUND_NOT_COMING_UP is the only state where the
> + cluster can actually be powered down.
> +
> + The parallelism of the inbound and outbound CPUs is observed by
> + the existence of two different paths from CLUSTER_GOING_DOWN/
> + INBOUND_NOT_COMING_UP (corresponding to GOING_DOWN in the basic
> + model) to CLUSTER_DOWN/INBOUND_COMING_UP (corresponding to
> + COMING_UP in the basic model). The second path avoids cluster
> + teardown completely.
> +
> + CLUSTER_UP/INBOUND_COMING_UP is equivalent to UP in the basic
> + model. The final transition to CLUSTER_UP/INBOUND_NOT_COMING_UP
> + is trivial and merely resets the state machine ready for the
> + next cycle.
> +
> + Details of the allowable transitions follow.
> +
> + The next state in each case is notated
> +
> + <cluster state>/<inbound state> (<transitioner>)
> +
> + where the <transitioner> is the side on which the transition
> + can occur; either the inbound or the outbound side.
> +
> +
> +CLUSTER_DOWN/INBOUND_NOT_COMING_UP:
> +
> + Next state: CLUSTER_DOWN/INBOUND_COMING_UP (inbound)
> + Conditions: none
> + Trigger events:
> +
> + a) an explicit hardware power-up operation, resulting
> + from a policy decision on another CPU;
> +
> + b) a hardware event, such as an interrupt.
> +
> +
> +CLUSTER_DOWN/INBOUND_COMING_UP:
> +
> + In this state, an inbound CPU sets up the cluster, including
> + enabling of hardware coherency at the cluster level and any
> + other operations (such as cache invalidation) which are required
> + in order to achieve this.
> +
> + The purpose of this state is to do sufficient cluster-level
> + setup to enable other CPUs in the cluster to enter coherency
> + safely.
> +
> + Next state: CLUSTER_UP/INBOUND_COMING_UP (inbound)
> + Conditions: cluster-level setup and hardware coherency complete
> + Trigger events: (spontaneous)
> +
> +
> +CLUSTER_UP/INBOUND_COMING_UP:
> +
> + Cluster-level setup is complete and hardware coherency is
> + enabled for the cluster. Other CPUs in the cluster can safely
> + enter coherency.
> +
> + This is a transient state, leading immediately to
> + CLUSTER_UP/INBOUND_NOT_COMING_UP. All other CPUs on the cluster
> + should consider treat these two states as equivalent.
> +
> + Next state: CLUSTER_UP/INBOUND_NOT_COMING_UP (inbound)
> + Conditions: none
> + Trigger events: (spontaneous)
> +
> +
> +CLUSTER_UP/INBOUND_NOT_COMING_UP:
> +
> + Cluster-level setup is complete and hardware coherency is
> + enabled for the cluster. Other CPUs in the cluster can safely
> + enter coherency.
> +
> + The cluster will remain in this state until a policy decision is
> + made to power the cluster down.
> +
> + Next state: CLUSTER_GOING_DOWN/INBOUND_NOT_COMING_UP (outbound)
> + Conditions: none
> + Trigger events: policy decision to power down the cluster
> +
> +
> +CLUSTER_GOING_DOWN/INBOUND_NOT_COMING_UP:
> +
> + An outbound CPU is tearing the cluster down. The selected CPU
> + must wait in this state until all CPUs in the cluster are in the
> + CPU_DOWN state.
> +
> + When all CPUs are in the CPU_DOWN state, the cluster can be torn
> + down, for example by cleaning data caches and exiting
> + cluster-level coherency.
> +
> + To avoid wasteful unnecessary teardown operations, the outbound
> + should check the inbound cluster state for asynchronous
> + transitions to INBOUND_COMING_UP. Alternatively, individual
> + CPUs can be checked for entry into CPU_COMING_UP or CPU_UP.
> +
> +
> + Next states:
> +
> + CLUSTER_DOWN/INBOUND_NOT_COMING_UP (outbound)
> + Conditions: cluster torn down and ready to power off
> + Trigger events: (spontaneous)
> +
> + CLUSTER_GOING_DOWN/INBOUND_COMING_UP (inbound)
> + Conditions: none
> + Trigger events:
> +
> + a) an explicit hardware power-up operation,
> + resulting from a policy decision on another
> + CPU;
> +
> + b) a hardware event, such as an interrupt.
> +
> +
> +CLUSTER_GOING_DOWN/INBOUND_COMING_UP:
> +
> + The cluster is (or was) being torn down, but another CPU has
> + come online in the meantime and is trying to set up the cluster
> + again.
> +
> + If the outbound CPU observes this state, it has two choices:
> +
> + a) back out of teardown, restoring the cluster to the
> + CLUSTER_UP state;
> +
> + b) finish tearing the cluster down and put the cluster
> + in the CLUSTER_DOWN state; the inbound CPU will
> + set up the cluster again from there.
> +
> + Choice (a) permits the removal of some latency by avoiding
> + unnecessary teardown and setup operations in situations where
> + the cluster is not really going to be powered down.
> +
> +
> + Next states:
> +
> + CLUSTER_UP/INBOUND_COMING_UP (outbound)
> + Conditions: cluster-level setup and hardware
> + coherency complete
> + Trigger events: (spontaneous)
> +
> + CLUSTER_DOWN/INBOUND_COMING_UP (outbound)
> + Conditions: cluster torn down and ready to power off
> + Trigger events: (spontaneous)
> +
> +
> +Last man and First man selection
> +--------------------------------
> +
> +The CPU which performs cluster tear-down operations on the outbound side
> +is commonly referred to as the "last man".
> +
> +The CPU which performs cluster setup on the inbound side is commonly
> +referred to as the "first man".
> +
> +The race avoidance algorithm documented above does not provide a
> +mechanism to choose which CPUs should play these roles.
> +
> +
> +Last man:
> +
> +When shutting down the cluster, all the CPUs involved are initially
> +executing Linux and hence coherent. Therefore, ordinary spinlocks can
> +be used to select a last man safely, before the CPUs become
> +non-coherent.
> +
> +
> +First man:
> +
> +Because CPUs may power up asynchronously in response to external wake-up
> +events, a dynamic mechanism is needed to make sure that only one CPU
> +attempts to play the first man role and do the cluster-level
> +initialisation: any other CPUs must wait for this to complete before
> +proceeding.
> +
> +Cluster-level initialisation may involve actions such as configuring
> +coherency controls in the bus fabric.
> +
> +The current implementation in bL_head.S uses a separate mutual exclusion
> +mechanism to do this arbitration. This mechanism is documented in
> +detail in vlocks.txt.
> +
> +
> +Features and Limitations
> +------------------------
> +
> +Implementation:
> +
> + The current ARM-based implementation is split between
> + arch/arm/common/bL_head.S (low-level inbound CPU operations) and
> + arch/arm/common/bL_entry.c (everything else):
> +
> + __bL_cpu_going_down() signals the transition of a CPU to the
> + CPU_GOING_DOWN state.
> +
> + __bL_cpu_down() signals the transition of a CPU to the CPU_DOWN
> + state.
> +
> + A CPU transitions to CPU_COMING_UP and then to CPU_UP via the
> + low-level power-up code in bL_head.S. This could
> + involve CPU-specific setup code, but in the current
> + implementation it does not.
> +
> + __bL_outbound_enter_critical() and __bL_outbound_leave_critical()
> + handle transitions from CLUSTER_UP to CLUSTER_GOING_DOWN
> + and from there to CLUSTER_DOWN or back to CLUSTER_UP (in
> + the case of an aborted cluster power-down).
> +
> + These functions are more complex than the __bL_cpu_*()
> + functions due to the extra inter-CPU coordination which
> + is needed for safe transitions at the cluster level.
> +
> + A cluster transitions from CLUSTER_DOWN back to CLUSTER_UP via
> + the low-level power-up code in bL_head.S. This
> + typically involves platform-specific setup code,
> + provided by the platform-specific power_up_setup
> + function registered via bL_cluster_sync_init.
> +
> +Deep topologies:
> +
> + As currently described and implemented, the algorithm does not
> + support CPU topologies involving more than two levels (i.e.,
> + clusters of clusters are not supported). The algorithm could be
> + extended by replicating the cluster-level states for the
> + additional topological levels, and modifying the transition
> + rules for the intermediate (non-outermost) cluster levels.
> +
> +
> +Colophon
> +--------
> +
> +Originally created and documented by Dave Martin for Linaro Limited, in
> +collaboration with Nicolas Pitre and Achin Gupta.
> +
Great write-up Dave!! I might have to do couple of more passes on it to
get overall idea, but surely this documentation is good start for
anybody reading/reviewing the big.LITTLE switcher code.
> +Copyright (C) 2012 Linaro Limited
> +Distributed under the terms of Version 2 of the GNU General Public
> +License, as defined in linux/COPYING.
> diff --git a/arch/arm/common/bL_entry.c b/arch/arm/common/bL_entry.c
> index 41de0622de..1ea4ec9df0 100644
> --- a/arch/arm/common/bL_entry.c
> +++ b/arch/arm/common/bL_entry.c
> @@ -116,3 +116,163 @@ int bL_cpu_powered_up(void)
> platform_ops->powered_up();
> return 0;
> }
> +
> +struct bL_sync_struct bL_sync;
> +
> +static void __sync_range(volatile void *p, size_t size)
> +{
> + char *_p = (char *)p;
> +
> + __cpuc_flush_dcache_area(_p, size);
> + outer_flush_range(__pa(_p), __pa(_p + size));
> + outer_sync();
> +}
> +
> +#define sync_mem(ptr) __sync_range(ptr, sizeof *(ptr))
> +
> +/*
/** as per kerneldoc.
> + * __bL_cpu_going_down: Indicates that the cpu is being torn down.
> + * This must be called at the point of committing to teardown of a CPU.
> + * The CPU cache (SCTRL.C bit) is expected to still be active.
> + */
> +void __bL_cpu_going_down(unsigned int cpu, unsigned int cluster)
> +{
> + bL_sync.clusters[cluster].cpus[cpu].cpu = CPU_GOING_DOWN;
> + sync_mem(&bL_sync.clusters[cluster].cpus[cpu].cpu);
> +}
> +
[..]
> diff --git a/arch/arm/common/bL_head.S b/arch/arm/common/bL_head.S
> index 9d351f2b4c..f7a64ac127 100644
> --- a/arch/arm/common/bL_head.S
> +++ b/arch/arm/common/bL_head.S
> @@ -7,11 +7,19 @@
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> * published by the Free Software Foundation.
> + *
> + *
> + * Refer to Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt
> + * for details of the synchronisation algorithms used here.
> */
>
> #include <linux/linkage.h>
> #include <asm/bL_entry.h>
>
> +.if BL_SYNC_CLUSTER_CPUS
> +.error "cpus must be the first member of struct bL_cluster_sync_struct"
> +.endif
> +
> .macro pr_dbg cpu, string
> #if defined(CONFIG_DEBUG_LL) && defined(DEBUG)
> b 1901f
> @@ -52,12 +60,82 @@ ENTRY(bL_entry_point)
> 2: pr_dbg r4, "kernel bL_entry_point\n"
>
> /*
> - * MMU is off so we need to get to bL_entry_vectors in a
> + * MMU is off so we need to get to various variables in a
> * position independent way.
> */
> adr r5, 3f
> - ldr r6, [r5]
> + ldmia r5, {r6, r7, r8}
> add r6, r5, r6 @ r6 = bL_entry_vectors
> + ldr r7, [r5, r7] @ r7 = bL_power_up_setup_phys
> + add r8, r5, r8 @ r8 = bL_sync
> +
> + mov r0, #BL_SYNC_CLUSTER_SIZE
> + mla r8, r0, r10, r8 @ r8 = bL_sync cluster base
> +
> + @ Signal that this CPU is coming UP:
> + mov r0, #CPU_COMING_UP
> + mov r5, #BL_SYNC_CPU_SIZE
> + mla r5, r9, r5, r8 @ r5 = bL_sync cpu address
> + strb r0, [r5]
> +
> + dsb
Do you really need above dsb(). With MMU off, the the store should any
way make it to the main memory, No ?
> +
> + @ At this point, the cluster cannot unexpectedly enter the GOING_DOWN
> + @ state, because there is at least one active CPU (this CPU).
> +
> + @ Check if the cluster has been set up yet:
> + ldrb r0, [r8, #BL_SYNC_CLUSTER_CLUSTER]
> + cmp r0, #CLUSTER_UP
> + beq cluster_already_up
> +
> + @ Signal that the cluster is being brought up:
> + mov r0, #INBOUND_COMING_UP
> + strb r0, [r8, #BL_SYNC_CLUSTER_INBOUND]
> +
> + dsb
Same comment.
Regards,
Santosh
^ permalink raw reply
* [kvmarm] [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Christoffer Dall @ 2013-01-11 17:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F04F2F.80306@arm.com>
On Fri, Jan 11, 2013 at 12:43 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 11/01/13 17:33, Christoffer Dall wrote:
>> On Fri, Jan 11, 2013 at 12:24 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> On 11/01/13 17:12, Russell King - ARM Linux wrote:
>>>> On Thu, Jan 10, 2013 at 04:06:45PM +0000, Marc Zyngier wrote:
>>>>> +int kvm_psci_call(struct kvm_vcpu *vcpu)
>>>>> +{
>>>>> + unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>>>> + unsigned long val;
>>>>> +
>>>>> + switch (psci_fn) {
>>>>> + case KVM_PSCI_FN_CPU_OFF:
>>>>> + kvm_psci_vcpu_off(vcpu);
>>>>> + val = KVM_PSCI_RET_SUCCESS;
>>>>> + break;
>>>>> + case KVM_PSCI_FN_CPU_ON:
>>>>> + val = kvm_psci_vcpu_on(vcpu);
>>>>> + break;
>>>>> + case KVM_PSCI_FN_CPU_SUSPEND:
>>>>> + case KVM_PSCI_FN_MIGRATE:
>>>>> + val = KVM_PSCI_RET_NI;
>>>>> + break;
>>>>> +
>>>>> + default:
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + *vcpu_reg(vcpu, 0) = val;
>>>>> + return 0;
>>>>> +}
>>>>
>>>> We were discussing recently on #kernel about kernel APIs and the way that
>>>> our integer-returning functions pretty much use 0 for success, and -errno
>>>> for failures, whereas our pointer-returning functions are a mess.
>>>>
>>>> And above we have something returning -1 to some other chunk of code outside
>>>> this compilation unit. That doesn't sound particularly clever to me.
>>>
>>> The original code used to return -EINVAL, see:
>>> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-January/004509.html
>>>
>>> Christoffer (Cc-ed) didn't like this, hence the -1. I'm happy to revert
>>> the code to its original state though.
>>>
>> I don't want to return -EINVAL, because for the rest of the KVM code
>> this would mean kill the guest.
>>
>> The convention used in other archs of KVM as well as for ARM is that
>> the handle_exit functions return:
>>
>> -ERRNO: Error, report this error to user space
>> 0: Everything is fine, but return to user space to let it do I/O
>> emulation and whatever it wants to do
>> 1: Everything is fine, return directly to the guest without going to user space
>
> That is assuming we propagate the handle_exit convention down to the
> leaf calls, and I object to that. The 3 possible values only apply to
> handle_exit, and we should keep that convention as local as possible,
> because this is the odd case.
I don't agree - it requires you to carefully follow a potentially deep
call trace to make sense of what it does, and worse, it's directly
misleading in the case of KVM/ARM. So either change it everywhere and
have a consistent calling convention or adhere to what we do elsewhere
and use the bool.
>
>> And then you do:
>> if (handle_something() == 0)
>> return 1;
>>
>> which I thought was confusing, so I said make the function a bool, to
>> avoid the confusion, like Rusty did for all the coprocessor emulation
>> functions.
>
> I don't see a compelling reason to propagate this convention to areas
> that do not require it. In the PSCI case, we have a basic
> handled/not-handled state, the later indicating the reason. The exit
> handling functions can convert the error codes to whatever the run loop
> requires.
>
again, that's why I suggest returning a bool instead. You just said
it: it's a basic handled/not-handled state. Why do you want to return
-EINVAL if that's not propogated anywhere?
If the return codes are local and do not map reasonably to error codes
and more complicated than a bool, I would vote for a #define
HVC_HANDLE_SUCCESS, HVC_HANDLE_ERROR, HVC_HANDLE_XXXXXX, ...
>> There are obviously other ways to handle the "return 1" case, like
>> having an extra state that you carry around, and we can change all the
>> code to do that, but I just don't think it's worth it, as we are in
>> fact quite close to the existing kernel API.
>
^ permalink raw reply
* [PATCH v4 07/14] dmaengine: add dma_request_slave_channel_compat()
From: Tony Lindgren @ 2013-01-11 17:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357883330-5364-8-git-send-email-mporter@ti.com>
* Matt Porter <mporter@ti.com> [130110 21:47]:
> Adds a dma_request_slave_channel_compat() wrapper which accepts
> both the arguments from dma_request_channel() and
> dma_request_slave_channel(). Based on whether the driver is
> instantiated via DT, the appropriate channel request call will be
> made.
>
> This allows for a much cleaner migration of drivers to the
> dmaengine DT API as platforms continue to be mixed between those
> that boot using DT and those that do not.
Cool, looks like the driver changes are quite minimal after this:
Acked-by: Tony Lindgren <tony@atomide.com>
>
> Suggested-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
> include/linux/dmaengine.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 9fd0c5b..64f9f69 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -1047,6 +1047,16 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx);
> struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
> struct dma_chan *net_dma_find_channel(void);
> #define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y)
> +static inline struct dma_chan
> +*dma_request_slave_channel_compat(dma_cap_mask_t mask, dma_filter_fn fn,
> + void *fn_param, struct device *dev,
> + char *name)
> +{
> + if (dev->of_node)
> + return dma_request_slave_channel(dev, name);
> + else
> + return dma_request_channel(mask, fn, fn_param);
> +}
>
> /* --- Helper iov-locking functions --- */
>
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCH v4 08/14] mmc: omap_hsmmc: convert to dma_request_slave_channel_compat()
From: Tony Lindgren @ 2013-01-11 17:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357883330-5364-9-git-send-email-mporter@ti.com>
* Matt Porter <mporter@ti.com> [130110 21:47]:
> Convert dmaengine channel requests to use
> dma_request_slave_channel_compat(). This supports the DT case of
> platforms requiring channel selection from either the OMAP DMA or
> the EDMA engine. AM33xx only boots from DT and is the only user
> implementing EDMA so in the !DT case we can default to the OMAP DMA
> filter.
Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
> drivers/mmc/host/omap_hsmmc.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index bc58078..e79b12d 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1915,14 +1915,20 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
> dma_cap_zero(mask);
> dma_cap_set(DMA_SLAVE, mask);
>
> - host->rx_chan = dma_request_channel(mask, omap_dma_filter_fn, &rx_req);
> + host->rx_chan =
> + dma_request_slave_channel_compat(mask, omap_dma_filter_fn,
> + &rx_req, &pdev->dev, "rx");
> +
> if (!host->rx_chan) {
> dev_err(mmc_dev(host->mmc), "unable to obtain RX DMA engine channel %u\n", rx_req);
> ret = -ENXIO;
> goto err_irq;
> }
>
> - host->tx_chan = dma_request_channel(mask, omap_dma_filter_fn, &tx_req);
> + host->tx_chan =
> + dma_request_slave_channel_compat(mask, omap_dma_filter_fn,
> + &tx_req, &pdev->dev, "tx");
> +
> if (!host->tx_chan) {
> dev_err(mmc_dev(host->mmc), "unable to obtain TX DMA engine channel %u\n", tx_req);
> ret = -ENXIO;
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCH v4 09/14] mmc: omap_hsmmc: set max_segs based on dma engine limitations
From: Tony Lindgren @ 2013-01-11 17:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357883330-5364-10-git-send-email-mporter@ti.com>
* Matt Porter <mporter@ti.com> [130110 21:47]:
> The EDMA DMAC has a hardware limitation that prevents supporting
> scatter gather lists with any number of segments. The DMA Engine
> API reports the maximum number of segments a channel can support
> via the optional dma_get_channel_caps() API. If the nr_segs
> capability is present, the value is used to configure mmc->max_segs
> appropriately.
Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
> drivers/mmc/host/omap_hsmmc.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index e79b12d..f74bd69 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1769,6 +1769,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
> const struct of_device_id *match;
> dma_cap_mask_t mask;
> unsigned tx_req, rx_req;
> + struct dmaengine_chan_caps *dma_chan_caps;
> struct pinctrl *pinctrl;
>
> match = of_match_device(of_match_ptr(omap_mmc_of_match), &pdev->dev);
> @@ -1935,6 +1936,11 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
> goto err_irq;
> }
>
> + /* Some DMA Engines only handle a limited number of SG segments */
> + dma_chan_caps = dma_get_channel_caps(host->rx_chan, DMA_DEV_TO_MEM);
> + if (dma_chan_caps && dma_chan_caps->seg_nr)
> + mmc->max_segs = dma_chan_caps->seg_nr;
> +
> /* Request IRQ for MMC operations */
> ret = request_irq(host->irq, omap_hsmmc_irq, 0,
> mmc_hostname(mmc), host);
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
From: Mark Rutland @ 2013-01-11 17:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111165053.GB1794@arm.com>
On Fri, Jan 11, 2013 at 04:50:53PM +0000, Catalin Marinas wrote:
> On Fri, Jan 11, 2013 at 01:40:22PM +0000, Santosh Shilimkar wrote:
> > On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > > Several bits in CNTKCTL reset to 0, including PL0VTEN. For platforms
> > > using the generic timer which wish to have a fast gettimeofday vDSO
> > > implementation, these bits must be set to 1 by the kernel. On other
> > > platforms, the bootloader might enable userspace access when we don't
> > > want it.
> > >
> > > This patch adds arch_counter_set_user_access, which sets the PL0 access
> > > permissions to that required by the platform. For arm, this currently
> > minor nit.
> > s/arm/ARM
>
> I think Mark meant arch/arm. Or we could call it AArch32 where we don't
> use this feature.
Indeed, I meant arch/arm. I'll try to be more consistent with that in future.
Mark.
^ permalink raw reply
* [PATCH v4 10/14] mmc: omap_hsmmc: add generic DMA request support to the DT binding
From: Tony Lindgren @ 2013-01-11 17:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357883330-5364-11-git-send-email-mporter@ti.com>
* Matt Porter <mporter@ti.com> [130110 21:47]:
> The binding definition is based on the generic DMA request binding.
Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
> .../devicetree/bindings/mmc/ti-omap-hsmmc.txt | 25 +++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> index ed271fc..826cc51 100644
> --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> @@ -20,8 +20,28 @@ ti,dual-volt: boolean, supports dual voltage cards
> ti,non-removable: non-removable slot (like eMMC)
> ti,needs-special-reset: Requires a special softreset sequence
> ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed
> +dmas: DMA controller phandle and DMA request value ordered pair
> +One tx and one rx pair is required.
> +dma-names: DMA request names. These strings correspond 1:1 with
> +the ordered pairs in dmas. The RX request must be "rx" and the
> +TX request must be "tx".
> +
> +Examples:
> +
> +[hwmod populated DMA resources]
> +
> + mmc1: mmc at 0x4809c000 {
> + compatible = "ti,omap4-hsmmc";
> + reg = <0x4809c000 0x400>;
> + ti,hwmods = "mmc1";
> + ti,dual-volt;
> + bus-width = <4>;
> + vmmc-supply = <&vmmc>; /* phandle to regulator node */
> + ti,non-removable;
> + };
> +
> +[generic DMA request binding]
>
> -Example:
> mmc1: mmc at 0x4809c000 {
> compatible = "ti,omap4-hsmmc";
> reg = <0x4809c000 0x400>;
> @@ -30,4 +50,7 @@ Example:
> bus-width = <4>;
> vmmc-supply = <&vmmc>; /* phandle to regulator node */
> ti,non-removable;
> + dmas = <&edma 24
> + &edma 25>;
> + dma-names = "tx", "rx";
> };
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCH v4 11/14] ARM: dts: add AM33XX MMC support
From: Tony Lindgren @ 2013-01-11 17:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357883330-5364-12-git-send-email-mporter@ti.com>
* Matt Porter <mporter@ti.com> [130110 21:47]:
> Adds AM33XX MMC support for am335x-bone, am335x-evm, and
> am335x-evmsk..
This one should be queued separately by Benoit:
Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Matt Porter <mporter@ti.com>
> ---
> arch/arm/boot/dts/am335x-bone.dts | 7 +++++++
> arch/arm/boot/dts/am335x-evm.dts | 7 +++++++
> arch/arm/boot/dts/am335x-evmsk.dts | 7 +++++++
> arch/arm/boot/dts/am33xx.dtsi | 28 ++++++++++++++++++++++++++++
> 4 files changed, 49 insertions(+)
>
> diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
> index 11b240c..a154ce0 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -120,6 +120,8 @@
> };
>
> ldo3_reg: regulator at 5 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> regulator-always-on;
> };
>
> @@ -136,3 +138,8 @@
> &cpsw_emac1 {
> phy_id = <&davinci_mdio>, <1>;
> };
> +
> +&mmc1 {
> + status = "okay";
> + vmmc-supply = <&ldo3_reg>;
> +};
> diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
> index d649644..2907da6 100644
> --- a/arch/arm/boot/dts/am335x-evm.dts
> +++ b/arch/arm/boot/dts/am335x-evm.dts
> @@ -232,6 +232,8 @@
> };
>
> vmmc_reg: regulator at 12 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> regulator-always-on;
> };
> };
> @@ -244,3 +246,8 @@
> &cpsw_emac1 {
> phy_id = <&davinci_mdio>, <1>;
> };
> +
> +&mmc1 {
> + status = "okay";
> + vmmc-supply = <&vmmc_reg>;
> +};
> diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts
> index f5a6162..f050c46 100644
> --- a/arch/arm/boot/dts/am335x-evmsk.dts
> +++ b/arch/arm/boot/dts/am335x-evmsk.dts
> @@ -244,7 +244,14 @@
> };
>
> vmmc_reg: regulator at 12 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> regulator-always-on;
> };
> };
> };
> +
> +&mmc1 {
> + status = "okay";
> + vmmc-supply = <&vmmc_reg>;
> +};
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index e711ffb..278b75d 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -235,6 +235,34 @@
> status = "disabled";
> };
>
> + mmc1: mmc at 48060000 {
> + compatible = "ti,omap3-hsmmc";
> + ti,hwmods = "mmc1";
> + ti,dual-volt;
> + ti,needs-special-reset;
> + dmas = <&edma 24
> + &edma 25>;
> + dma-names = "tx", "rx";
> + status = "disabled";
> + };
> +
> + mmc2: mmc at 481d8000 {
> + compatible = "ti,omap3-hsmmc";
> + ti,hwmods = "mmc2";
> + ti,needs-special-reset;
> + dmas = <&edma 2
> + &edma 3>;
> + dma-names = "tx", "rx";
> + status = "disabled";
> + };
> +
> + mmc3: mmc at 47810000 {
> + compatible = "ti,omap3-hsmmc";
> + ti,hwmods = "mmc3";
> + ti,needs-special-reset;
> + status = "disabled";
> + };
> +
> wdt2: wdt at 44e35000 {
> compatible = "ti,omap3-wdt";
> ti,hwmods = "wd_timer2";
> --
> 1.7.9.5
>
^ permalink raw reply
* [REPOST PATCH 0/2] Add automatic bus number support for i2c busses with device tree
From: Doug Anderson @ 2013-01-11 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1353522362-25519-2-git-send-email-dianders@chromium.org>
This was suggested by Mark Brown in response to a patch for adding
this functionality only for the s3c2410 bus:
https://lkml.org/lkml/2012/11/20/681
I have also modified the i2c-pxa driver to use this new functionality.
As Haojian Zhuang has Acked this patch, I believe that this has now
been tested.
Reposting (with Haojian Zhuang's ack) in the hopes that a maintainer
will pick it up. :)
Doug Anderson (2):
i2c-core: dt: Pick i2c bus number from i2c alias if present
i2c: pxa: Use i2c-core to get bus number now
drivers/i2c/busses/i2c-pxa.c | 8 +---
drivers/i2c/i2c-core.c | 105 ++++++++++++++++++++++++++++++-----------
2 files changed, 78 insertions(+), 35 deletions(-)
--
1.7.7.3
^ permalink raw reply
* [REPOST PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present
From: Doug Anderson @ 2013-01-11 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357927027-4857-1-git-send-email-dianders@chromium.org>
This allows you to get the equivalent functionality of
i2c_add_numbered_adapter() with all data in the device tree and no
special case code in your driver. This is a common device tree
technique.
For quick reference, the FDT syntax for using an alias to provide an
ID looks like:
aliases {
i2c0 = &i2c_0;
i2c1 = &i2c_1;
};
Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
drivers/i2c/i2c-core.c | 105 +++++++++++++++++++++++++++++++++++-------------
1 files changed, 77 insertions(+), 28 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e388590..a60ed6d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -921,13 +921,81 @@ out_list:
}
/**
+ * i2c_get_number_from_dt - get the adapter number based on dt alias
+ * @adap: the adapter to look at
+ *
+ * Check whether there's an alias in the FDT that gives an ID for this i2c
+ * device. Use an alias like "i2c<nr>", like:
+ * aliases {
+ * i2c0 = &i2c_0;
+ * i2c1 = &i2c_1;
+ * };
+ *
+ * Returns the ID if found. If no alias is found returns -1.
+ */
+static int i2c_get_number_from_dt(struct i2c_adapter *adap)
+{
+ struct device *dev = &adap->dev;
+ int id;
+
+ if (!dev->of_node)
+ return -1;
+
+ id = of_alias_get_id(dev->of_node, "i2c");
+ if (id < 0)
+ return -1;
+ return id;
+}
+
+/**
+ * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
+ * @adap: the adapter to register (with adap->nr initialized)
+ * Context: can sleep
+ *
+ * See i2c_add_numbered_adapter() for details.
+ */
+static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)
+{
+ int id;
+ int status;
+
+ /* Handled by wrappers */
+ BUG_ON(adap->nr == -1);
+
+ if (adap->nr & ~MAX_IDR_MASK)
+ return -EINVAL;
+
+retry:
+ if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
+ return -ENOMEM;
+
+ mutex_lock(&core_lock);
+ /* "above" here means "above or equal to", sigh;
+ * we need the "equal to" result to force the result
+ */
+ status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
+ if (status == 0 && id != adap->nr) {
+ status = -EBUSY;
+ idr_remove(&i2c_adapter_idr, id);
+ }
+ mutex_unlock(&core_lock);
+ if (status == -EAGAIN)
+ goto retry;
+
+ if (status == 0)
+ status = i2c_register_adapter(adap);
+ return status;
+}
+
+/**
* i2c_add_adapter - declare i2c adapter, use dynamic bus number
* @adapter: the adapter to add
* Context: can sleep
*
* This routine is used to declare an I2C adapter when its bus number
- * doesn't matter. Examples: for I2C adapters dynamically added by
- * USB links or PCI plugin cards.
+ * doesn't matter or when its bus number is specified by an dt alias.
+ * Examples of bases when the bus number doesn't matter: I2C adapters
+ * dynamically added by USB links or PCI plugin cards.
*
* When this returns zero, a new bus number was allocated and stored
* in adap->nr, and the specified adapter became available for clients.
@@ -937,6 +1005,12 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
{
int id, res = 0;
+ id = i2c_get_number_from_dt(adapter);
+ if (id >= 0) {
+ adapter->nr = id;
+ return _i2c_add_numbered_adapter(adapter);
+ }
+
retry:
if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
return -ENOMEM;
@@ -983,34 +1057,9 @@ EXPORT_SYMBOL(i2c_add_adapter);
*/
int i2c_add_numbered_adapter(struct i2c_adapter *adap)
{
- int id;
- int status;
-
if (adap->nr == -1) /* -1 means dynamically assign bus id */
return i2c_add_adapter(adap);
- if (adap->nr & ~MAX_IDR_MASK)
- return -EINVAL;
-
-retry:
- if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
- return -ENOMEM;
-
- mutex_lock(&core_lock);
- /* "above" here means "above or equal to", sigh;
- * we need the "equal to" result to force the result
- */
- status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
- if (status == 0 && id != adap->nr) {
- status = -EBUSY;
- idr_remove(&i2c_adapter_idr, id);
- }
- mutex_unlock(&core_lock);
- if (status == -EAGAIN)
- goto retry;
-
- if (status == 0)
- status = i2c_register_adapter(adap);
- return status;
+ return _i2c_add_numbered_adapter(adap);
}
EXPORT_SYMBOL_GPL(i2c_add_numbered_adapter);
--
1.7.7.3
^ permalink raw reply related
* [REPOST PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now
From: Doug Anderson @ 2013-01-11 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357927027-4857-1-git-send-email-dianders@chromium.org>
The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
present" adds support for automatically picking the bus number based
on the alias ID. Remove the now unnecessary code from i2c-pxa that
did the same thing.
Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
drivers/i2c/busses/i2c-pxa.c | 8 +-------
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..8ee9fa0 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1053,16 +1053,10 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
struct device_node *np = pdev->dev.of_node;
const struct of_device_id *of_id =
of_match_device(i2c_pxa_dt_ids, &pdev->dev);
- int ret;
if (!of_id)
return 1;
- ret = of_alias_get_id(np, "i2c");
- if (ret < 0) {
- dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
- return ret;
- }
- pdev->id = ret;
+ pdev->id = -1;
if (of_get_property(np, "mrvl,i2c-polling", NULL))
i2c->use_pio = 1;
if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
--
1.7.7.3
^ permalink raw reply related
* [kvmarm] [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Russell King - ARM Linux @ 2013-01-11 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANM98qLRsdFGxU6S5q2EqtCAmK8fOnbGWi8qXBktyuRkSaPAJg@mail.gmail.com>
On Fri, Jan 11, 2013 at 12:48:45PM -0500, Christoffer Dall wrote:
> again, that's why I suggest returning a bool instead. You just said
> it: it's a basic handled/not-handled state. Why do you want to return
> -EINVAL if that's not propogated anywhere?
We have a well established principle throughout the kernel interfaces that
functions will return positive values for success and an appropriate -ve
errno for failure.
We *certainly* hate functions which return 0 for failure and non-zero
for success - it makes review a real pain because you start seeing code
doing this:
if (!function()) {
deal_with_failure();
}
and you have to then start looking at the function to properly understand
what it's return semantics are.
We have more than enough proof already that this doesn't work: people
don't care to understand what the return values from functions mean.
You only have to do an audit of a few of the IS_ERR_OR_NULL() uses to
find that out, and you quickly realise that people just use what they
_think_ is the right test and which happens to pass their very simple
testing at the time.
We've avoided major problems so far in the kernel by having most of the
integer-returning functions following the established principle, and
that's good. I really really think that there must be a _very_ good
reason, and overwhelming reason to deviate from the established
principle in any large project.
^ permalink raw reply
* [PATCH 02/11] ARM: nomadik: initial devicetree support
From: Mark Rutland @ 2013-01-11 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111170411.320D73E20EC@localhost>
On Fri, Jan 11, 2013 at 05:04:11PM +0000, Grant Likely wrote:
> On Tue, 8 Jan 2013 09:57:29 +0000, Mark Rutland <mark.rutland@arm.com> wrote:
> > Hi,
> >
> > I have a couple of comments on the dts{,i} portion.
> >
> > On Mon, Jan 07, 2013 at 10:55:59PM +0000, Linus Walleij wrote:
> > > Support basic device tree boot on the Nomadik. Implement the
> > > support in the cpu file with the intent of deleting the board
> > > files later. At this stage IRQ controllers, system timer,
> > > l2x0 cache, UARTs and thus console boot is fully functional.
> > > Patch out the code adding devices by initcalls for now so
> > > as not to disturb the boot.
> > >
> > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > > ---
> > > arch/arm/boot/dts/Makefile | 1 +
> > > arch/arm/boot/dts/ste-nomadik-s8815.dts | 18 ++++++
> > > arch/arm/boot/dts/ste-nomadik-stn8815.dtsi | 77 +++++++++++++++++++++++++
> > > arch/arm/mach-nomadik/board-nhk8815.c | 4 ++
> > > arch/arm/mach-nomadik/cpu-8815.c | 90 ++++++++++++++++++++++++++++++
> > > arch/arm/mach-nomadik/cpu-8815.h | 1 +
> > > arch/arm/mach-nomadik/i2c-8815nhk.c | 5 ++
> > > 7 files changed, 196 insertions(+)
> > > create mode 100644 arch/arm/boot/dts/ste-nomadik-s8815.dts
> > > create mode 100644 arch/arm/boot/dts/ste-nomadik-stn8815.dtsi
> > >
> > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > > index e44da40..7bc5ba73 100644
> > > --- a/arch/arm/boot/dts/Makefile
> > > +++ b/arch/arm/boot/dts/Makefile
> > > @@ -100,6 +100,7 @@ dtb-$(CONFIG_ARCH_MXS) += imx23-evk.dtb \
> > > imx28-m28evk.dtb \
> > > imx28-sps1.dtb \
> > > imx28-tx28.dtb
> > > +dtb-$(CONFIG_ARCH_NOMADIK) += ste-nomadik-s8815.dtb
> > > dtb-$(CONFIG_ARCH_OMAP2PLUS) += omap2420-h4.dtb \
> > > omap3-beagle.dtb \
> > > omap3-beagle-xm.dtb \
> > > diff --git a/arch/arm/boot/dts/ste-nomadik-s8815.dts b/arch/arm/boot/dts/ste-nomadik-s8815.dts
> > > new file mode 100644
> > > index 0000000..b698bc2
> > > --- /dev/null
> > > +++ b/arch/arm/boot/dts/ste-nomadik-s8815.dts
> > > @@ -0,0 +1,18 @@
> > > +/*
> > > + * Device Tree for the ST-Ericsson Nomadik S8815 board
> > > + * Produced by Calao Systems
> > > + */
> > > +
> > > +/dts-v1/;
> > > +/include/ "ste-nomadik-stn8815.dtsi"
> > > +
> > > +/ {
> > > + model = "ST-Ericsson Nomadik S8815";
> > > + compatible = "stericsson,nomadik";
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> >
> > Given there are addresses and sizes in the dtsi (but none in the dts), should
> > #address-cells and #size-cells not be in the dtsi?
> >
> > > +
> > > + chosen {
> > > + bootargs = "root=/dev/ram0 console=ttyAMA1,115200n8 earlyprintk";
> > > + };
> > > +};
> > > diff --git a/arch/arm/boot/dts/ste-nomadik-stn8815.dtsi b/arch/arm/boot/dts/ste-nomadik-stn8815.dtsi
> > > new file mode 100644
> > > index 0000000..81284c5
> > > --- /dev/null
> > > +++ b/arch/arm/boot/dts/ste-nomadik-stn8815.dtsi
> > > @@ -0,0 +1,77 @@
> > > +/*
> > > + * Device Tree for the ST-Ericsson Nomadik 8815 STn8815 SoC
> > > + */
> > > +/include/ "skeleton.dtsi"
> > > +
> > > +/ {
> > > + memory {
> > > + reg = <0x00000000 0x04000000>;
> > > + reg = <0x08000000 0x04000000>;
> >
> > I believe that's not valid (duplicate property name).
> >
> > Judging by other dts files under arch/arm/boot/dts, these should be two nodes,
> > with their unit addresses set (and possibly device_type, which I see is set for
> > some dts files, but not all).
>
> One node should be just fine if the following form is used:
>
> memory {
> reg = <0x00000000 0x04000000>,
> <0x08000000 0x04000000>;
> };
>
That also makes sense.
> >
> > Maybe I've misunderstood how this is laid out, but I can't see where we get a
> > cpus node from in the board's dts. Has this just been forgotten?
>
> A cpus node isn't required if it doesn't provide any non-discoverable
> information.
>
Seeing the discussion around the Tegra #CPUs detection code, I'd think we
should have one:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/140209.html
Thanks,
Mark.
^ permalink raw reply
* [PATCH V2 1/2] ARM: OMAP2+: Prepare for device-tree PMU support
From: Tony Lindgren @ 2013-01-11 17:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1355766548-5702-2-git-send-email-jon-hunter@ti.com>
* Jon Hunter <jon-hunter@ti.com> [121217 09:51]:
> If device-tree is present, then do not create the PMU device from within
> the OMAP specific PMU code. This is required to allow device-tree to
> create the PMU device from the PMU device-tree node.
>
> PMU is not currently supported for OMAP4430 (due to a dependency on
> having a cross-trigger interface driver) and so ensure that this
> indicated on boot with or without device-tree.
Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---
> arch/arm/mach-omap2/pmu.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pmu.c b/arch/arm/mach-omap2/pmu.c
> index eb78ae7..1a0799c 100644
> --- a/arch/arm/mach-omap2/pmu.c
> +++ b/arch/arm/mach-omap2/pmu.c
> @@ -11,6 +11,8 @@
> * the Free Software Foundation; either version 2 of the License, or
> * (at your option) any later version.
> */
> +#include <linux/of.h>
> +
> #include <asm/pmu.h>
>
> #include "soc.h"
> @@ -64,6 +66,15 @@ static int __init omap_init_pmu(void)
> unsigned oh_num;
> char **oh_names;
>
> + /* XXX Remove this check when the CTI driver is available */
> + if (cpu_is_omap443x()) {
> + pr_info("ARM PMU: not yet supported on OMAP4430 due to missing CTI driver\n");
> + return 0;
> + }
> +
> + if (of_have_populated_dt())
> + return 0;
> +
> /*
> * To create an ARM-PMU device the following HWMODs
> * are required for the various OMAP2+ devices.
> @@ -76,9 +87,6 @@ static int __init omap_init_pmu(void)
> if (cpu_is_omap443x()) {
> oh_num = ARRAY_SIZE(omap4430_pmu_oh_names);
> oh_names = omap4430_pmu_oh_names;
> - /* XXX Remove the next two lines when CTI driver available */
> - pr_info("ARM PMU: not yet supported on OMAP4430 due to missing CTI driver\n");
> - return 0;
> } else if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
> oh_num = ARRAY_SIZE(omap3_pmu_oh_names);
> oh_names = omap3_pmu_oh_names;
> --
> 1.7.10.4
>
^ permalink raw reply
* [PATCH V2 2/2] ARM: dts: OMAP2+: Add PMU nodes
From: Tony Lindgren @ 2013-01-11 17:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F01AAD.8020003@ti.com>
* Jon Hunter <jon-hunter@ti.com> [130111 06:02]:
> Hi Benoit,
>
> On 01/11/2013 07:23 AM, Benoit Cousson wrote:
> > Hi Jon,
> >
> > On 12/17/2012 06:49 PM, Jon Hunter wrote:
> >> Add PMU nodes for OMAP2, OMAP3 and OMAP4460 devices.
> >>
> >> Please note that the node for OMAP4460 has been placed in a separate
> >> header file for OMAP4460, because the node is not compatible with
> >> OMAP4430. The node for OMAP4430 is not included because PMU is not
> >> currently supported on OMAP4430 due to the absence of a cross-trigger
> >> interface driver.
> >>
> >> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> >
> > I've just applied this patch in my for_3.9/dts branch.
> >
> > I'm wondering if there is any dependency with the previous patch? If
> > Tony ack it I can take it as well.
>
> I have been thinking about the best way to handle that. May be best for
> you to take both if Tony can ack the first.
Sounds good to me, I've acked it now.
Tony
^ permalink raw reply
* [PATCH] ARM: let CPUs not being able to run in ARM mode enter in THUMB mode
From: Jonathan Austin @ 2013-01-11 18:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111160753.GC23505@n2100.arm.linux.org.uk>
Hi Russell, Uwe
On 11/01/13 16:07, Russell King - ARM Linux wrote:
> On Fri, Jan 11, 2013 at 12:39:57PM +0100, Uwe Kleine-K?nig wrote:
>> +# Select this if your CPU doesn't support the 32 bit ARM instructions.
>> +config THUMBONLY_CPU
>> + bool
>> + select THUMB2_KERNEL
>> + select ARM_THUMB
>
> Hmm, not convinced this is the best solution. Yes, fine for there to be
> a THUMBONLY_CPU option, _but_ not the select statements onto user visible
> symbols. We can get this instead by:
I'm curious what it is about having the select statements onto user
visible symbols that isn't good. Is it just that it will manipulate
things underneath the users' feet?
(I'm not disagreeing! I like your proposal, but when I saw the
original patch this didn't raise flags with me, so I'd like to
understand the reasons)
> config CPU_ARM
> bool
>
> config CPU_THUMB
> bool
The only thing I might add is that we don't want people confusing
the selection of THUMB2_KERNEL with exclusively selecting CPU_THUMB
(which should be very rare). The latter would imply the former, but
in almost all cases where THUMB2_KERNEL is selected we should also
have CPU_ARM selected...
However things fall out, I'd still like to be *unable* to use
THUMBONLY_CPU *with* an MMU, as it is a clear sign (at least for the
foreseeable future) that something has been misunderstood.
Jonny
^ permalink raw reply
* [PATCH 06/16] ARM: b.L: generic SMP secondary bringup and hotplug support
From: Santosh Shilimkar @ 2013-01-11 18:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-7-git-send-email-nicolas.pitre@linaro.org>
On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
> Now that the b.L power API is in place, we can use it for SMP secondary
> bringup and CPU hotplug in a generic fashion.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/common/Makefile | 2 +-
> arch/arm/common/bL_platsmp.c | 79 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 80 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm/common/bL_platsmp.c
>
> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> index 894c2ddf9b..59b36db7cc 100644
> --- a/arch/arm/common/Makefile
> +++ b/arch/arm/common/Makefile
> @@ -15,4 +15,4 @@ obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o
> obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o
> obj-$(CONFIG_FIQ_GLUE) += fiq_glue.o fiq_glue_setup.o
> obj-$(CONFIG_FIQ_DEBUGGER) += fiq_debugger.o
> -obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o vlock.o
> +obj-$(CONFIG_BIG_LITTLE) += bL_head.o bL_entry.o bL_platsmp.o vlock.o
> diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c
> new file mode 100644
> index 0000000000..0acb9f4685
> --- /dev/null
> +++ b/arch/arm/common/bL_platsmp.c
> @@ -0,0 +1,79 @@
> +/*
> + * linux/arch/arm/mach-vexpress/bL_platsmp.c
> + *
> + * Created by: Nicolas Pitre, November 2012
> + * Copyright: (C) 2012 Linaro Limited
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Code to handle secondary CPU bringup and hotplug for the bL power API.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/smp.h>
> +
> +#include <asm/bL_entry.h>
> +#include <asm/smp_plat.h>
> +#include <asm/hardware/gic.h>
> +
> +static void __init simple_smp_init_cpus(void)
> +{
> + set_smp_cross_call(gic_raise_softirq);
> +}
> +
> +static int __cpuinit bL_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> + unsigned int pcpu, pcluster, ret;
> + extern void secondary_startup(void);
> +
> + pcpu = cpu_logical_map(cpu) & 0xff;
> + pcluster = (cpu_logical_map(cpu) >> 8) & 0xff;
> + pr_debug("%s: logical CPU %d is physical CPU %d cluster %d\n",
> + __func__, cpu, pcpu, pcluster);
> +
> + bL_set_entry_vector(pcpu, pcluster, NULL);
> + ret = bL_cpu_power_up(pcpu, pcluster);
> + if (ret)
> + return ret;
> + bL_set_entry_vector(pcpu, pcluster, secondary_startup);
> + gic_raise_softirq(cpumask_of(cpu), 0);
> + sev();
softirq() should be enough to break a CPU if it is in standby with
wfe state. Is that additional sev() needed here ?
Regards,
Santosh
^ permalink raw reply
* [PATCH 2/5] ARM: mach-shmobile: fix memory size for kota2_defconfig
From: Olof Johansson @ 2013-01-11 18:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357888626-4946-3-git-send-email-horms+renesas@verge.net.au>
On Thu, Jan 10, 2013 at 11:17 PM, Simon Horman
<horms+renesas@verge.net.au> wrote:
> The CONFIG_MEMORY_SIZE value is interpreted as a 32 bit integer, which
> makes sense on a system without PAE. It appears that a trailing 0 was
> appended to the value and after some testing it appears that 0x1e000000 is
> the correct value.
>
> Without this patch, building kota2_defconfig results in:
>
> /home/arnd/linux-arm/arch/arm/kernel/setup.c:790:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Olof Johansson <olof@lixom.net>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Olof Johansson <olof@lixom.net>
Or is this the only fix you have right now, so I should just apply it directly?
-Olof
^ permalink raw reply
* [kvmarm] [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Christoffer Dall @ 2013-01-11 18:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111175714.GK23505@n2100.arm.linux.org.uk>
On Fri, Jan 11, 2013 at 12:57 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Jan 11, 2013 at 12:48:45PM -0500, Christoffer Dall wrote:
>> again, that's why I suggest returning a bool instead. You just said
>> it: it's a basic handled/not-handled state. Why do you want to return
>> -EINVAL if that's not propogated anywhere?
>
> We have a well established principle throughout the kernel interfaces that
> functions will return positive values for success and an appropriate -ve
> errno for failure.
>
> We *certainly* hate functions which return 0 for failure and non-zero
> for success - it makes review a real pain because you start seeing code
> doing this:
>
> if (!function()) {
> deal_with_failure();
> }
>
and you'd certainly not find that in any of the KVM/ARM - that would
be despicable.
> and you have to then start looking at the function to properly understand
> what it's return semantics are.
>
> We have more than enough proof already that this doesn't work: people
> don't care to understand what the return values from functions mean.
> You only have to do an audit of a few of the IS_ERR_OR_NULL() uses to
> find that out, and you quickly realise that people just use what they
> _think_ is the right test and which happens to pass their very simple
> testing at the time.
>
> We've avoided major problems so far in the kernel by having most of the
> integer-returning functions following the established principle, and
> that's good. I really really think that there must be a _very_ good
> reason, and overwhelming reason to deviate from the established
> principle in any large project.
The _very_ good reason here, is that we have two success cases: return
to guest and return to user space. As I said, we can save this state
in another bit somewhere and change all the KVM/ARM code to do so, but
the KVM guys back then would like to use the same convention as other
KVM archs.
I would prefer not having to change all that KVM/ARM code at this
point, but of course, if you insists, I will have to do that.
BUT, returning -EINVAL should indicate an actual error. This is not
the case for the concrete psci example, the case is simply that the
number in r0 does not fall within the psci range, and therefore could
potentially be handled by something else, and using -EINVAL as a
fall-through to the next value is equally weird and misleading.
An alternative is to do something like foo(..., &handled), but I think
using a bool as the function type is perfect here: what is the problem
with that again? Certainly it wouldn't be the only boolean function in
the kernel.
-Christoffer
^ permalink raw reply
* [PATCH 08/16] ARM: bL_platsmp.c: make sure the GIC interface of a dying CPU is disabled
From: Santosh Shilimkar @ 2013-01-11 18:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-9-git-send-email-nicolas.pitre@linaro.org>
On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
> Otherwise there might be some interrupts or IPIs becoming pending and the
> CPU will not enter low power mode when doing a WFI. The effect of this
> is a CPU that loops back into the kernel, go through the first man
> election, signals itself as alive, and prevent the cluster from being
> shut down.
>
> This could benefit from a better solution.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/common/bL_platsmp.c | 1 +
> arch/arm/common/gic.c | 6 ++++++
> arch/arm/include/asm/hardware/gic.h | 2 ++
> 3 files changed, 9 insertions(+)
>
> diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c
> index 0ae44123bf..6a3b251b97 100644
> --- a/arch/arm/common/bL_platsmp.c
> +++ b/arch/arm/common/bL_platsmp.c
> @@ -68,6 +68,7 @@ static void __ref bL_cpu_die(unsigned int cpu)
> pcpu = mpidr & 0xff;
> pcluster = (mpidr >> 8) & 0xff;
> bL_set_entry_vector(pcpu, pcluster, NULL);
> + gic_cpu_if_down();
So for a case where CPU still don't power down for some reason even
after CPU interface is disabled, can not listen to and SGI or PPI.
Not sure if this happens on big.LITTLE but i have seen one such issue
on Cortex-A9 based SOC.
Regards
Santosh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox