* [PATCH 0/2 v2] at91/ssc: fixes on ASoC tree for 3.8
From: Olof Johansson @ 2013-01-11 18:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1357912706.git.nicolas.ferre@atmel.com>
Hi,
On Fri, Jan 11, 2013 at 6:08 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
> This material was designed to enter Mark's fixes queue, but as discussed with
> Olof, we can imagine merging everything through arm-soc or split the series (of
> 2 patches) and let them progress upstream separated (option that I do not like
> even if I know that the consequences are not so dramatic).
> So please, Olof, if you feel confortable with this series, tell us what you
> prefer and we will make our best to make this material go forward...
You're setting yourself up for awkward merges. The driver change is
strongly dependent on the device tree change by failing probe unless
the device tree update is there, while before this patch, it still
worked.
If you instead use the dts update if it's there, you can merge the two
through independent paths, and later make it mandatory to use the
pinctrl specification. That way you avoid these complicated merge
scenarios where you have to send your platform code through a
subsystem tree instead.
But to be honest, I don't think this is a fix, it's a feature that you
just didn't include in time for the merge window. I don't really see
them as appropriate 3.8 material at this point.
So, nack on this series. Please make the driver change non-dependent
on the new dtsi contents, and merge that through the ASoC tree. Then
the dtsi update can go through arm-soc, and later on you can make it
mandatory.
-Olof
^ permalink raw reply
* [PATCH 14/14] ARM: tegra: trimslice: Initialize PCIe from DT
From: Thierry Reding @ 2013-01-11 18:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50EF5537.6080602@wwwdotorg.org>
On Thu, Jan 10, 2013 at 04:56:39PM -0700, Stephen Warren wrote:
> On 01/09/2013 01:43 PM, Thierry Reding wrote:
> > With the device tree support in place, probe the PCIe controller from
> > the device tree and remove the corresponding workaround in the board
> > file.
>
> Thierry, there are a couple things missing from this patch; I include a
> fixup for you to squash in, but also see beyond that for a problem.
>
> > arch/arm/boot/dts/tegra20-trimslice.dts | 26 +++++++++++++++++++++-----
> > 1 file changed, 21 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts b/arch/arm/boot/dts/tegra20-trimslice.dts
> > index ebb4c17..41fc45b 100644
> > --- a/arch/arm/boot/dts/tegra20-trimslice.dts
> > +++ b/arch/arm/boot/dts/tegra20-trimslice.dts
> > @@ -303,12 +303,10 @@
> >
> > pcie-controller {
> > status = "okay";
> > + pex-clk-supply = <&pci_clk_reg>;
> > + vdd-supply = <&pci_vdd_reg>;
> >
> > - pci at 0 {
> > - status = "okay";
> > - };
> > -
> > - pci at 1 {
> > + pci at 1,0 {
> > status = "okay";
> > };
> > };
> > @@ -366,6 +364,24 @@
> > regulator-max-microvolt = <1800000>;
> > regulator-always-on;
> > };
> > +
> > + pci_clk_reg: regulator at 2 {
> > + compatible = "regulator-fixed";
> > + reg = <2>;
> > + regulator-name = "pci_clk";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-always-on;
> > + };
> > +
> > + pci_vdd_reg: regulator at 3 {
> > + compatible = "regulator-fixed";
> > + reg = <3>;
> > + regulator-name = "pci_vdd";
> > + regulator-min-microvolt = <1050000>;
> > + regulator-max-microvolt = <1050000>;
> > + regulator-always-on;
> > + };
> > };
> >
> > sound {
> > --
> > 1.7.10.4
>
> However, PCIe still doesn't work on TrimSlice; the device enumeration
> fails. The log is below:
>
> > [ 0.790425] tegra-pcie 80003000.pcie-controller: link 0 down, retrying
> > [ 0.801351] tegra-pcie 80003000.pcie-controller: PCI host bridge to bus 0000:00
> > [ 0.808945] pci_bus 0000:00: root bus resource [io 0x82000000-0x8200ffff]
> > [ 0.816109] pci_bus 0000:00: root bus resource [mem 0xa0000000-0xafffffff]
> > [ 0.823303] pci_bus 0000:00: root bus resource [mem 0xb0000000-0xbfffffff pref]
> > [ 0.830893] pci_bus 0000:00: root bus resource [bus 00-ff]
> > [ 0.836728] pci 0000:00:01.0: [10de:0bf0] type 01 class 0x060000
> > [ 0.843097] pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot D3cold
> > [ 0.850047] PCI: bus0: Fast back to back transfers disabled
> > [ 0.855853] pci 0000:00:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
> > [ 0.864308] tegra-pcie 80003000.pcie-controller: AXI response decoding error, signature: 10010001
> > [ 0.873489] tegra-pcie 80003000.pcie-controller: FPCI address: fe10010000
>
> ^^^ For some reason, the config register access fails.
>
> > [ 0.880760] PCI: bus1: Fast back to back transfers enabled
> > [ 0.886482] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
> > [ 0.893381] pci 0000:00:01.0: PCI bridge to [bus 01]
> > [ 0.898577] PCI: enabling device 0000:00:01.0 (0140 -> 0143)
> > [ 0.904535] pci 0000:00:01.0: nv_msi_ht_cap_quirk didn't locate host bridge
>
> With plain next-20130109, the device enumeration succeeds.
That's very strange. I haven't seen that on my setup. Looks like I need
to dig out the TrimSlice again and debug this. It'll take me a while and
there are plenty of other requested changes already, so if I find a fix
I can roll it into the next version.
Thanks for testing,
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130111/ab94f169/attachment-0001.sig>
^ permalink raw reply
* [PATCH 02/16] ARM: b.L: introduce the CPU/cluster power API
From: Santosh Shilimkar @ 2013-01-11 18:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301111312180.6300@xanadu.home>
On Saturday 12 January 2013 12:03 AM, Nicolas Pitre wrote:
> On Fri, 11 Jan 2013, Santosh Shilimkar wrote:
>
>> 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.
>
> There are two things to consider here:
>
> 1) The CPU is suspended. CPU state is lost. Next interrupt to wake up
> the CPU will make it restart from the reset vector and re-entry in
> the kernel will happen via bL_entry_point to deal with the various
> cluster issues, to eventually resume kernel code via cpu_resume.
> Obviously, the machine specific backend code would have set the
> bL_entry_point address in its machine specific reset vector in
> advance.
This is the successful case and in that case you will anyway not hit the
BUG.
>
> 2) An interrupt comes along before the CPU is effectively suspended, say
> right before the backend code executes a WFI to shut the CPU down.
> The CPU and possibly cluster state was already set for being powered
> off. We cannot simply return at this point as caches are off, the
> CPU is not coherent with the rest of the system anymore, etc. So if
> the platform specific backend ever returns, say because the final WFI
> exited, then we have to go through the same arbitration process to
> restore the CPU and cluster state as if that was a hard reset. Hence
> the cpu_reset call to loop back into bL_entry_point.
>
This is the one I was thinking. Enabling C bit and SMP bit should be
enough for CPU to get back to right state since the CPU has not lost
the context all the registers including SP is intact and CPU should
be able to resume.
> In either cases, we simply cannot ever return from bL_cpu_suspend()
> directly. Of course, the caller is expected to have used
> bL_set_entry_vector() beforehand, most probably with cpu_resume as
> argument.
>
The above might get complicated if when above situation happens on
last CPU where even CCI gets disabled and then adding the rever code
for all of that may not be worth. You approach is much safer.
Thanks for explaining it further.
Regards,
Santosh
^ permalink raw reply
* [kvmarm] [PATCH v5.1 0/2] KVM: ARM: Rename KVM_SET_DEVICE_ADDRESS
From: Marcelo Tosatti @ 2013-01-11 18:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <709FA96C-A3EF-489C-B0E9-59EEA7F8E62F@virtualopensystems.com>
On Fri, Jan 11, 2013 at 02:26:51AM -0500, Christoffer Dall wrote:
> On 10/01/2013, at 20.10, Scott Wood <scottwood@freescale.com> wrote:
>
> > On 01/10/2013 06:35:02 PM, Marcelo Tosatti wrote:
> >> On Thu, Jan 10, 2013 at 04:40:12PM -0600, Scott Wood wrote:
> >> > On 01/10/2013 04:28:01 PM, Marcelo Tosatti wrote:
> >> > >Or just have KVM_SET_PPC_DEVICE_ADDRESS. Is there a downside to that?
> >> >
> >> > Besides the above, and my original complaint that it shouldn't be
> >> > specific to addresses?
> >> >
> >> > -Scott
> >> I did not really grasp that ('shouldnt be specific to addresses'), but
> >> anyway.
> >
> > A device may have other configuration parameters that need to be set,
> > besides addresses. PPC MPIC will require information about the vendor
> > and version, for example.
> >
> >> OK, can you write down your proposed improvements to the interface?
> >> In case you have something ready, otherwise there is time pressure
> >> to merge the ARM port.
> >
> > My original request was just to change the name to something like
> > KVM_SET_DEVICE_CONFIG or KVM_SET_DEVICE_ATTR, and not make the id
> > encoding architecture-specific (preferably, separate into a "device id"
> > field and an "attribute id" field rather than using bitfields). Actual
> > values for device id could be architecture-specific (or there could be a
> > global enumeration), and attribute id values would be device-specific.
> >
> > Alex suggested that an ideal interface might accept values larger than 64
> > bits, though I think it's good enough -- there are currently no proposed
> > uses that need more than 64 bits for a single attribute (unlike ONE_REG),
> > and if it is needed, such configuration could be split up between
> > multiple attributes, or the attribute could specify that "value" be a
> > userspace pointer to the actual data (as with ONE_REG).
> >
> > Here's a writeup (the ARM details would go under ARM/vGIC-specific
> > documentation):
> >
> > 4.80 KVM_SET_DEVICE_ATTR
> >
> > Capability: KVM_CAP_SET_DEVICE_ATTR
> > Type: vm ioctl
> > Parameters: struct kvm_device_attr (in)
> > Returns: 0 on success, -1 on error
> > Errors:
> > ENODEV: The device id is unknown
> > ENXIO: Device not supported on current system
> > Other errors may be returned by specific devices and attributes.
> >
> > struct kvm_device_attr {
> > __u32 device;
> > __u32 attr;
> > __u64 value;
> > };
> >
> > Specify an attribute of a device emulated or directly exposed by the
> > kernel, which the host kernel needs to know about. The device field is an
> > architecture-specific identifier for a specific device. The attr field
> > is a device-specific identifier for a specific attribute. Individual
> > attributes may have particular requirements for when they can and cannot
> > be set.
> >
> >> That is, if you have interest/energy to spend in a possibly reusable
> >> interface, as long as that does not delay integration of the ARM code,
> >> i don't think the ARM people will mind that.
> >
> > The impression I've been given is that just about any change will delay
> > the integration at this point. If that's the case, and everyone's OK
> > with having an interface that is deprecated on arrival, then fine.
>
>
> That is not entirely the case, but there wasn't event agreement on this revised API, and we didn't want to wait for weeks until a decision was made. Alex suggested we use a DEV_REG API similar to the ONE_REG API, but I am quite strongly against having such an API for this specific purpose as it is too semantically distant to what we do on ARM. (Having a DEV_REG API for other purposes might be fine though).
>
> So I have no problem with your suggestion, although we could consider having a size and addr field in the struct instead and be slightly more extensible. But I don't feel strongly about it.
>
> If we can agree on Scott's suggestion or with my modification (Alex, Gleb, Marcelo ?) then I'll change the KVM/ARM patches to use this and resend them right away. But we have to agree now!
>
> If not, I really think we should keep things as they are now, as we're really discussing idealistic scenarios here, and the ARM patches have been out of tree for long enough. As Marcelo pointed out, the benefits of the perfect API are really minimal.
>
> -Christoffer--
Can you make KVM_SET_ARM_DEVICE address extensible?
Add some reserved space and a flags field.
^ permalink raw reply
* [PATCH 03/16] ARM: b.L: introduce helpers for platform coherency exit/setup
From: Santosh Shilimkar @ 2013-01-11 18:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111180757.GI1966@linaro.org>
On Friday 11 January 2013 11:37 PM, Dave Martin wrote:
> On Fri, Jan 11, 2013 at 11:16:18PM +0530, Santosh Shilimkar wrote:
>
> [...]
>
>>> +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.
>
> Thanks for reading through it. Partly, this was insurance against me
> forgetting how the code worked in between writing and posting it...
> but this is all quite subtle code, so it felt important to document
> it thoroughly.
>
>>
>>> +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.
>
> Does kerneldoc not require the comment to be specially formatted?
>
> I haven't played with that, so far.
>
>>
>>> + * __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
>
> The short answer is "maybe not". Some of the barriers can be
> eliminated; some can be demoted to DSBs. Others may be required but
> unnecessarily duplicated e.g., between bL_head.S and vlock.S.
>
>> any way make it to the main memory, No ?
>
> Yes, but this raises issues about precisely what the architecture
> guarantees about memory ordering in these scenarios. The only obvious
> thing about that is that it's non-obvious.
>
Well at least ARM documents clearly says the memory accesses will be
treated as strongly ordered with MMU OFF and that means they expect
to make it to main memory.
> Strongly-Ordered memory is not quite the same as having explicit
> barriers everywhere.
>
> I need to have a careful think, but it should be possible to optimise
> a bit here.
>
If the CCI comes in between that rule and if it needs a barrier to let
it flush is WB to main memory then thats a different story.
Anyway thanks for the answer.
Regards
Santosh
^ permalink raw reply
* [PATCH 02/16] ARM: b.L: introduce the CPU/cluster power API
From: Nicolas Pitre @ 2013-01-11 18:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F04B4F.2070300@ti.com>
On Fri, 11 Jan 2013, Santosh Shilimkar wrote:
> 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.
There are two things to consider here:
1) The CPU is suspended. CPU state is lost. Next interrupt to wake up
the CPU will make it restart from the reset vector and re-entry in
the kernel will happen via bL_entry_point to deal with the various
cluster issues, to eventually resume kernel code via cpu_resume.
Obviously, the machine specific backend code would have set the
bL_entry_point address in its machine specific reset vector in
advance.
2) An interrupt comes along before the CPU is effectively suspended, say
right before the backend code executes a WFI to shut the CPU down.
The CPU and possibly cluster state was already set for being powered
off. We cannot simply return at this point as caches are off, the
CPU is not coherent with the rest of the system anymore, etc. So if
the platform specific backend ever returns, say because the final WFI
exited, then we have to go through the same arbitration process to
restore the CPU and cluster state as if that was a hard reset. Hence
the cpu_reset call to loop back into bL_entry_point.
In either cases, we simply cannot ever return from bL_cpu_suspend()
directly. Of course, the caller is expected to have used
bL_set_entry_vector() beforehand, most probably with cpu_resume as
argument.
Nicolas
^ permalink raw reply
* [PATCH v4 00/14] DMA Engine support for AM33XX
From: Matt Porter @ 2013-01-11 18:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201301111140.41584.arnd@arndb.de>
On Fri, Jan 11, 2013 at 11:40:41AM +0000, Arnd Bergmann wrote:
> On Friday 11 January 2013, Matt Porter wrote:
> > The approach taken is similar to how OMAP DMA is being converted to
> > DMA Engine support. With the functional EDMA private API already
> > existing in mach-davinci/dma.c, we first move that to an ARM common
> > area so it can be shared. Adding DT and runtime PM support to the
> > private EDMA API implementation allows it to run on AM33xx. AM33xx
> > only boots using DT so we leverage Jon's generic DT DMA helpers to
> > register EDMA DMAC with the of_dma framework and then add support
> > for calling the dma_request_slave_channel() API to both the mmc
> > and spi drivers.
>
> I think this looks very good. What I wonder is whether we should
> make the non-DT parts of the dmaengine driver compile-time
> conditional on CONFIG_ATAGS though, so the slave drivers don't
> have a link-time dependency on the dmaengine driver's
> omap_dma_filter_fn symbol when building without ATAGS support.
We have tightly coupled the link-time dependency for
omap_dma_filter_fn by going down the path of using
dma_request_slave_channel_compat() as Tony suggested to avoid extra
ifdefry.
That dependency will go away naturally if all the "legacy" OMAP platforms
were required to only boot from DT...just as a newly added SoCs are.
Are you suggesting unwinding the _compat() approach?
-Matt
^ permalink raw reply
* [PATCH 01/16] ARM: b.L: secondary kernel entry code
From: Santosh Shilimkar @ 2013-01-11 18:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301111301300.6300@xanadu.home>
On Friday 11 January 2013 11:40 PM, Nicolas Pitre wrote:
> On Fri, 11 Jan 2013, Santosh Shilimkar wrote:
>
>> On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
>>> +ENTRY(bL_entry_point)
>>> +
>>> + THUMB( adr r12, BSYM(1f) )
>>> + THUMB( bx r12 )
>>> + THUMB( .thumb )
>>> +1:
>>> + mrc p15, 0, r0, c0, c0, 5
>>> + ubfx r9, r0, #0, #4 @ r9 = cpu
>>> + ubfx r10, r0, #8, #4 @ r10 = cluster
>>> + mov r3, #BL_CPUS_PER_CLUSTER
>>> + mla r4, r3, r10, r9 @ r4 = canonical CPU index
>>> + cmp r4, #(BL_CPUS_PER_CLUSTER * BL_NR_CLUSTERS)
>>> + blo 2f
>>> +
>>> + /* We didn't expect this CPU. Try to make it quiet. */
>>> +1: wfi
>>> + wfe
>>
>> Why do you need a wfe followed by wif ?
>> Just curious.
>
> If the WFI doesn't work because an interrupt is pending then the WFE
> might work better. But as I mentioned before, this is not intended to
> be used for other purposes than "we're really screwed so at least let's
> try to cheaply quieten this CPU" case.
>
Thanks for clarification.
Regards
Santosh
^ permalink raw reply
* [PATCH 15/16] ARM: vexpress/dcscb: handle platform coherency exit/setup and CCI
From: Santosh Shilimkar @ 2013-01-11 18:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-16-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>
>
> Add the required code to properly handle race free platform coherency exit
> to the DCSCB power down method.
>
> The power_up_setup callback is used to enable the CCI interface for
> the cluster being brought up. This must be done in assembly before
> the kernel environment is entered.
>
> Thanks to Achin Gupta and Nicolas Pitre for their help and
> contributions.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
[..]
> diff --git a/arch/arm/mach-vexpress/dcscb.c b/arch/arm/mach-vexpress/dcscb.c
> index 59b690376f..95a2d0df20 100644
> --- a/arch/arm/mach-vexpress/dcscb.c
> +++ b/arch/arm/mach-vexpress/dcscb.c
> @@ -15,6 +15,7 @@
> #include <linux/spinlock.h>
> #include <linux/errno.h>
> #include <linux/vexpress.h>
> +#include <linux/arm-cci.h>
>
> #include <asm/bL_entry.h>
> #include <asm/proc-fns.h>
> @@ -104,6 +105,8 @@ static void dcscb_power_down(void)
> pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
> BUG_ON(cpu >= 4 || cluster >= 2);
>
> + __bL_cpu_going_down(cpu, cluster);
> +
> arch_spin_lock(&dcscb_lock);
> dcscb_use_count[cpu][cluster]--;
> if (dcscb_use_count[cpu][cluster] == 0) {
> @@ -111,6 +114,7 @@ static void dcscb_power_down(void)
> rst_hold |= cpumask;
> if (((rst_hold | (rst_hold >> 4)) & cluster_mask) == cluster_mask) {
> rst_hold |= (1 << 8);
> + BUG_ON(__bL_cluster_state(cluster) != CLUSTER_UP);
> last_man = true;
> }
> writel(rst_hold, dcscb_base + RST_HOLD0 + cluster * 4);
> @@ -124,35 +128,71 @@ static void dcscb_power_down(void)
> skip_wfi = true;
> } else
> BUG();
> - arch_spin_unlock(&dcscb_lock);
>
> - /*
> - * Now let's clean our L1 cache and shut ourself down.
> - * If we're the last CPU in this cluster then clean L2 too.
> - */
> -
> - /*
> - * A15/A7 can hit in the cache with SCTLR.C=0, so we don't need
> - * a preliminary flush here for those CPUs. At least, that's
> - * the theory -- without the extra flush, Linux explodes on
> - * RTSM (maybe not needed anymore, to be investigated)..
> - */
> - flush_cache_louis();
> - cpu_proc_fin();
> + if (last_man && __bL_outbound_enter_critical(cpu, cluster)) {
> + arch_spin_unlock(&dcscb_lock);
>
> - if (!last_man) {
> - flush_cache_louis();
> - } else {
> + /*
> + * Flush all cache levels for this cluster.
> + *
> + * A15/A7 can hit in the cache with SCTLR.C=0, so we don't need
> + * a preliminary flush here for those CPUs. At least, that's
> + * the theory -- without the extra flush, Linux explodes on
> + * RTSM (maybe not needed anymore, to be investigated).
> + */
> flush_cache_all();
> + cpu_proc_fin(); /* disable allocation into internal caches*/
I see now. In previous patch I missed the cpu_proc_fin() which clears
C bit
> + flush_cache_all();
> +
> + /*
> + * This is a harmless no-op. On platforms with a real
> + * outer cache this might either be needed or not,
> + * depending on where the outer cache sits.
> + */
> outer_flush_all();
> +
> + /* Disable local coherency by clearing the ACTLR "SMP" bit: */
> + asm volatile (
> + "mrc p15, 0, ip, c1, c0, 1 \n\t"
> + "bic ip, ip, #(1 << 6) @ clear SMP bit \n\t"
> + "mcr p15, 0, ip, c1, c0, 1 \n\t"
> + "isb \n\t"
> + "dsb"
> + : : : "ip" );
> +
> + /*
> + * Disable cluster-level coherency by masking
> + * incoming snoops and DVM messages:
> + */
> + disable_cci(cluster);
> +
> + __bL_outbound_leave_critical(cluster, CLUSTER_DOWN);
> + } else {
> + arch_spin_unlock(&dcscb_lock);
> +
> + /*
> + * Flush the local CPU cache.
> + *
> + * A15/A7 can hit in the cache with SCTLR.C=0, so we don't need
> + * a preliminary flush here for those CPUs. At least, that's
> + * the theory -- without the extra flush, Linux explodes on
> + * RTSM (maybe not needed anymore, to be investigated).
> + */
This is expected if the entire code is not in one stack frame and the
additional flush is needed to avoid possible stack corruption. This
issue has been discussed in past on the list.
> + flush_cache_louis();
> + cpu_proc_fin(); /* disable allocation into internal caches*/
> + flush_cache_louis();
> +
> + /* Disable local coherency by clearing the ACTLR "SMP" bit: */
> + asm volatile (
> + "mrc p15, 0, ip, c1, c0, 1 \n\t"
> + "bic ip, ip, #(1 << 6) @ clear SMP bit \n\t"
> + "mcr p15, 0, ip, c1, c0, 1 \n\t"
> + "isb \n\t"
> + "dsb"
> + : : : "ip" );
> }
>
> - /* Disable local coherency by clearing the ACTLR "SMP" bit: */
> - asm volatile (
> - "mrc p15, 0, ip, c1, c0, 1 \n\t"
> - "bic ip, ip, #(1 << 6) @ clear SMP bit \n\t"
> - "mcr p15, 0, ip, c1, c0, 1"
> - : : : "ip" );
> + __bL_cpu_down(cpu, cluster);
>
> /* Now we are prepared for power-down, do it: */
> if (!skip_wfi)
Regards,
Santosh
^ permalink raw reply
* [PATCH 07/14] usb: ehci-omap: Instantiate PHY devices if required
From: Alan Stern @ 2013-01-11 18:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F037C9.40306@ti.com>
On Fri, 11 Jan 2013, Roger Quadros wrote:
> Apart from what you mentioned I did some more trivial changes. e.g.
>
> + !IS_ENABLED(CONFIG_USB_EHCI_HCD_OMAP) && \
> instead of
> + !defined(CONFIG_USB_EHCI_HCD_OMAP) && \
Ah, that's a very good catch. There's another entry needing the same
thing. I'll put that in a separate preliminary patch, and also put
the ehci->priv addition in there instead of including it with the
ehci-mxc update.
> use ehci_hcd_omap_driver instead of ehci_omap_driver
Now fixed.
> I tried using ehci->priv in ehci-omap driver but noticed that the
> private data gets corrupted after the EHCI controller is running and has
> enumerated a few devices.
>
> If I disable USB_DEBUG then things are fine. Could it be possible
> that someone is overflowing data when USB_DEBUG is enabled?
>
> My implementation is pasted below. (May contain whitespace errors due to
> MS exchange). Patch 2 attached in case.
>
> What was happening there is that omap_priv->phy was not the same during
> remove() as it was set to during probe().
I don't understand -- your second patch doesn't use ehci->priv at all.
How can the private data be getting corrupted?
Below is an updated version of your second patch. You'll need to
resolve one or two merge errors because it's not based on the same
starting point as yours. (And it totally omits the part affecting
usb-omap.h.) But it will show you what needs to be done in order to use
ehci->priv.
> Would be nice if you could check if the same happens with ehci-mxc.
I can't -- I don't have an ARM-based system. But if you still see
problems, I can test with ehci-pci.
Alan Stern
Index: usb-3.7/drivers/usb/host/ehci-omap.c
===================================================================
--- usb-3.7.orig/drivers/usb/host/ehci-omap.c
+++ usb-3.7/drivers/usb/host/ehci-omap.c
@@ -69,6 +69,10 @@ static const char hcd_name[] = "ehci-oma
/*-------------------------------------------------------------------------*/
+struct omap_ehci_hcd {
+ struct usb_phy **phy; /* one PHY for each port */
+ int nports;
+};
static inline void ehci_write(void __iomem *base, u32 reg, u32 val)
{
@@ -177,7 +181,8 @@ static void disable_put_regulator(
static struct hc_driver __read_mostly ehci_omap_hc_driver;
static const struct ehci_driver_overrides ehci_omap_overrides __initdata = {
- .reset = omap_ehci_init,
+ .reset = omap_ehci_init,
+ .extra_priv_size = sizeof(struct omap_ehci_hcd),
};
/**
@@ -193,6 +198,8 @@ static int ehci_hcd_omap_probe(struct pl
struct ehci_hcd_omap_platform_data *pdata = dev->platform_data;
struct resource *res;
struct usb_hcd *hcd;
+ struct omap_ehci_hcd *omap_hcd;
+ struct usb_phy *phy;
void __iomem *regs;
int ret = -ENODEV;
int irq;
@@ -207,6 +214,11 @@ static int ehci_hcd_omap_probe(struct pl
return -ENODEV;
}
+ if (!pdata) {
+ dev_err(dev, "Missing platform data\n");
+ return -ENODEV;
+ }
+
irq = platform_get_irq_byname(pdev, "ehci-irq");
if (irq < 0) {
dev_err(dev, "EHCI irq failed\n");
@@ -238,8 +250,24 @@ static int ehci_hcd_omap_probe(struct pl
hcd->rsrc_len = resource_size(res);
hcd->regs = regs;
+ omap_hcd = (struct omap_ehci_hcd *) (hcd_to_ehci(hcd))->priv;
+
+ omap_hcd->nports = pdata->nports;
+ i = sizeof(struct usb_phy *) * omap_hcd->nports;
+ omap_hcd->phy = devm_kzalloc(&pdev->dev, i, GFP_KERNEL);
+ if (!omap_hcd->phy) {
+ dev_err(dev, "Memory allocation failed\n");
+ ret = -ENOMEM;
+ goto err_alloc_phy;
+ }
+
+ platform_set_drvdata(pdev, hcd);
+
/* get ehci regulator and enable */
- for (i = 0 ; i < OMAP3_HS_USB_PORTS ; i++) {
+ for (i = 0 ; i < omap_hcd->nports ; i++) {
+ struct platform_device *phy_pdev;
+ struct usbhs_phy_config *phy_config;
+
if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY) {
pdata->regulator[i] = NULL;
continue;
@@ -253,6 +281,33 @@ static int ehci_hcd_omap_probe(struct pl
} else {
regulator_enable(pdata->regulator[i]);
}
+
+ /* instantiate PHY */
+ if (!pdata->phy_config[i]) {
+ dev_dbg(dev, "missing phy_config for port %d\n", i);
+ continue;
+ }
+
+ phy_config = pdata->phy_config[i];
+ phy_pdev = platform_device_register_data(&pdev->dev,
+ phy_config->name, i, phy_config->pdata,
+ phy_config->pdata_size);
+ if (IS_ERR(phy_pdev)) {
+ dev_dbg(dev, "error creating PHY device for port %d\n",
+ i);
+ }
+
+ phy = usb_get_phy_from_dev(&phy_pdev->dev);
+ if (IS_ERR(phy)) {
+ dev_dbg(dev, "could not get USB PHY for port %d\n", i);
+ platform_device_unregister(phy_pdev);
+ continue;
+ }
+
+ usb_phy_init(phy);
+ omap_hcd->phy[i] = phy;
+ /* bring PHY out of suspend */
+ usb_phy_set_suspend(omap_hcd->phy[i], 0);
}
pm_runtime_enable(dev);
@@ -282,6 +336,14 @@ static int ehci_hcd_omap_probe(struct pl
err_pm_runtime:
disable_put_regulator(pdata);
pm_runtime_put_sync(dev);
+ for (i = 0 ; i < omap_hcd->nports ; i++) {
+ phy = omap_hcd->phy[i];
+ if (!phy)
+ continue;
+ platform_device_unregister(to_platform_device(phy->dev));
+ }
+
+err_alloc_phy:
usb_put_hcd(hcd);
err_io:
@@ -300,13 +362,26 @@ err_io:
*/
static int ehci_hcd_omap_remove(struct platform_device *pdev)
{
- struct device *dev = &pdev->dev;
- struct usb_hcd *hcd = dev_get_drvdata(dev);
- struct ehci_hcd_omap_platform_data *pdata = dev->platform_data;
+ struct device *dev = &pdev->dev;
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+ struct omap_ehci_hcd *omap_hcd;
+ int i;
usb_remove_hcd(hcd);
disable_put_regulator(dev->platform_data);
iounmap(hcd->regs);
+
+ omap_hcd = (struct omap_ehci_hcd *) (hcd_to_ehci(hcd))->priv;
+ for (i = 0; i < omap_hcd->nports; i++) {
+ struct usb_phy *phy = omap_hcd->phy[i];
+
+ if (!phy)
+ continue;
+
+ usb_phy_shutdown(phy);
+ platform_device_unregister(to_platform_device(phy->dev));
+ }
+
usb_put_hcd(hcd);
pm_runtime_put_sync(dev);
^ permalink raw reply
* [PATCH v3 0/9] Migrate Tegra to common clock framework
From: Stephen Warren @ 2013-01-11 18:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2615017.79obOGziy2@ax5200p>
On 01/11/2013 08:59 AM, Marc Dietrich wrote:
> On Friday 11 January 2013 13:40:03 Prashant Gaikwad wrote:
>> On Thursday 10 January 2013 02:14 AM, Stephen Warren wrote:
>>> On 01/09/2013 10:34 AM, Stephen Warren wrote:
...
>>> Your changes don't actually cause the driver to break though, since it
>>> abuses clk_get_sys() to retrieve clocks under a different driver name,
>>> which matches what the clock driver provides. However, I think you
>>> should also include the following patch at the end of your series to fix
>>> this up, so the clock looking happens through device tree:
>>>
>>>> diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
>>>> index d8826ed..6d44076 100644
>>>> --- a/drivers/staging/nvec/nvec.c
>>>> +++ b/drivers/staging/nvec/nvec.c
>>>> @@ -770,7 +770,7 @@ static int tegra_nvec_probe(struct platform_device
>>>> *pdev)>>
>>>> return -ENODEV;
>>>>
>>>> }
>>>>
>>>> - i2c_clk = clk_get_sys("tegra-i2c.2", "div-clk");
>>>> + i2c_clk = clk_get(&pdev->dev, "div-clk");
>>>>
>>>> if (IS_ERR(i2c_clk)) {
>>>>
>>>> dev_err(nvec->dev, "failed to get controller clock\n");
>>>> return -ENODEV;
>>
>> Included in the latest patches sent.
>
> em, not yet in V4.
It's in V2 of the other series Prashant posted which sits on top of the
CCF rework series.
> Maybe you can also adjust the TODO (2nd entry) file now
> that this issue is fixed.
I'll try to remember to repost an updated version of the patch which
does that...
^ permalink raw reply
* [PATCH v4,03/14] ARM: edma: add AM33XX support to the private EDMA API
From: Matt Porter @ 2013-01-11 18:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9fda86bbbec3444393fd5a2dc5135a5e@DFLE73.ent.ti.com>
On Fri, Jan 11, 2013 at 06:14:27PM +0000, Lars Poeschel wrote:
> Hi Matt,
>
> On Friday 11 January 2013 at 06:48:39, Matt Porter wrote:
>
> > diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
> > index a3d189d..1951d63 100644
> > --- a/arch/arm/common/edma.c
> > +++ b/arch/arm/common/edma.c
> > @@ -24,6 +24,13 @@
> > #include <linux/platform_device.h>
> > #include <linux/io.h>
> > #include <linux/slab.h>
> > +#include <linux/edma.h>
> > +#include <linux/err.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_dma.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/pm_runtime.h>
>
> You add the include linux/of_dma.h here, but maybe you missed to add the file
> itself. It is not in Linus v3.8-rc3 tree, not in the patches yours depend on
> listed and not in your patchset.
of_dma.h comes in with the dependencies listed in the cover letter.
Particularly, Vinod's dt_dmaengine branch in slave-dma.git contains Jon
Hunter's OF DMA helpers series.
-Matt
^ permalink raw reply
* [PATCH 13/16] drivers: misc: add ARM CCI support
From: Santosh Shilimkar @ 2013-01-11 18:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-14-git-send-email-nicolas.pitre@linaro.org>
On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>
> On ARM multi-cluster systems coherency between cores running on
> different clusters is managed by the cache-coherent interconnect (CCI).
> It allows broadcasting of TLB invalidates and memory barriers and it
> guarantees cache coherency at system level.
>
> This patch enables the basic infrastructure required in Linux to
> handle and programme the CCI component. The first implementation is
> based on a platform device, its relative DT compatible property and
> a simple programming interface.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> drivers/misc/Kconfig | 3 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/arm-cci.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/arm-cci.h | 30 ++++++++++++++
How about 'drivers/bus/' considering CCI is an interconnect bus (though
for coherency)
> 4 files changed, 141 insertions(+)
> create mode 100644 drivers/misc/arm-cci.c
> create mode 100644 include/linux/arm-cci.h
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index b151b7c1bd..30d5be1ad2 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -499,6 +499,9 @@ config USB_SWITCH_FSA9480
> stereo and mono audio, video, microphone and UART data to use
> a common connector port.
>
> +config ARM_CCI
You might want add depends on ARM big.LITTTLE otherwise it will
break build for other arch's with random configurations.
[..]
> diff --git a/drivers/misc/arm-cci.c b/drivers/misc/arm-cci.c
> new file mode 100644
> index 0000000000..f329c43099
> --- /dev/null
> +++ b/drivers/misc/arm-cci.c
> @@ -0,0 +1,107 @@
> +/*
> + * CCI support
> + *
> + * Copyright (C) 2012 ARM Ltd.
> + * Author: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> + *
> + * 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.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/arm-cci.h>
> +
> +#define CCI400_EAG_OFFSET 0x4000
> +#define CCI400_KF_OFFSET 0x5000
> +
> +#define DRIVER_NAME "CCI"
> +struct cci_drvdata {
> + void __iomem *baseaddr;
> + spinlock_t lock;
> +};
> +
> +static struct cci_drvdata *info;
> +
> +void disable_cci(int cluster)
> +{
> + u32 cci_reg = cluster ? CCI400_KF_OFFSET : CCI400_EAG_OFFSET;
> + writel_relaxed(0x0, info->baseaddr + cci_reg);
> +
> + while (readl_relaxed(info->baseaddr + 0xc) & 0x1)
> + ;
> +}
> +EXPORT_SYMBOL_GPL(disable_cci);
> +
Is more functionality going to be added for CCI driver. Having this
much of driver code for just a disable_cci() functions seems like
overkill.
Regards
Santosh
^ permalink raw reply
* [kvmarm] [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Christoffer Dall @ 2013-01-11 18:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F05562.3070107@arm.com>
On Fri, Jan 11, 2013 at 1:09 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 11/01/13 17:48, Christoffer Dall wrote:
>> 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.
>
> Sorry, but I do not buy this.
>
> In this particular case, the meaning of the value returned has nothing
> to do with the handle_exit convention. We never return to user space,
> let alone signaling an error.
>
But that is by no means obvious.
> Trying to force all the code in this convention makes it actually harder
> to review, because this is the exception in the kernel. This is why I
> suggest keeping the handle_exit return convention at this exact point,
> and not impose it on anything else.
>
Ok, then come up with a nicer convention, but using -EINVAL to mean I
did not handle this one, maybe someone else will handle it, and it's
not an error is definitely not easy to read. What then if another
function called from the same caller could actually return an error,
would you have a switch case on the return value checking for EINVAL
vs. EFAULT?
I repeat: what's the problem with a bool for this particular case,
which would make it look like all the other sub-calls of handle exit
functions - could we be consistent here?
-Christoffer
^ permalink raw reply
* [kvmarm] [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Christoffer Dall @ 2013-01-11 18:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111181407.GM23505@n2100.arm.linux.org.uk>
On Fri, Jan 11, 2013 at 1:14 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Jan 11, 2013 at 01:07:31PM -0500, Christoffer Dall wrote:
>> 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.
>
> Can you please credit me for not objecting to returning 0/1 to have
> different success meanings. What I'm merely objecting to is that
> "return -1" statement in the code (notice the negative sign.)
Sorry if I misunderstood you. Yes, the return -1 has to be changed.
-Christoffer
^ permalink raw reply
* [PATCH v4, 03/14] ARM: edma: add AM33XX support to the private EDMA API
From: Lars Poeschel @ 2013-01-11 18:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357883330-5364-4-git-send-email-mporter@ti.com>
Hi Matt,
On Friday 11 January 2013 at 06:48:39, Matt Porter wrote:
> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
> index a3d189d..1951d63 100644
> --- a/arch/arm/common/edma.c
> +++ b/arch/arm/common/edma.c
> @@ -24,6 +24,13 @@
> #include <linux/platform_device.h>
> #include <linux/io.h>
> #include <linux/slab.h>
> +#include <linux/edma.h>
> +#include <linux/err.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_dma.h>
> +#include <linux/of_irq.h>
> +#include <linux/pm_runtime.h>
You add the include linux/of_dma.h here, but maybe you missed to add the file
itself. It is not in Linus v3.8-rc3 tree, not in the patches yours depend on
listed and not in your patchset.
Regards,
Lars
^ permalink raw reply
* [kvmarm] [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Russell King - ARM Linux @ 2013-01-11 18:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANM98q+mUcEs+ggxa1rgRqCgWUOnnL=BXQprxgd2548Y+dDjAw@mail.gmail.com>
On Fri, Jan 11, 2013 at 01:07:31PM -0500, Christoffer Dall wrote:
> 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.
Can you please credit me for not objecting to returning 0/1 to have
different success meanings. What I'm merely objecting to is that
"return -1" statement in the code (notice the negative sign.)
^ permalink raw reply
* [PATCH 01/14] mfd: omap-usb-host: Consolidate OMAP USB-HS platform data
From: Tony Lindgren @ 2013-01-11 18:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50EFDE10.9080902@ti.com>
* Roger Quadros <rogerq@ti.com> [130111 01:43]:
> Tony,
>
> On 01/11/2013 01:45 AM, Tony Lindgren wrote:
> > * Roger Quadros <rogerq@ti.com> [130110 08:54]:
> >> Let's have a single platform data structure for the OMAP's High-Speed
> >> USB host subsystem instead of having 3 separate ones i.e. one for
> >> board data, one for USB Host (UHH) module and one for USB-TLL module.
> >>
> >> This makes the code much simpler and avoids creating multiple copies of
> >> platform data.
> >
> > I can apply just this patch alone into an immutable branch that
> > we all can merge in as needed as long as we have acks for the USB
> > and MFD parts.
> >
> > Or does this one need to be changed based on Alan's comments
> > on the EHCI lib related changes?
> >
>
> This does not depend on EHCI lib based changes but it depends on the
> OMAP USB Host cleanup series posted earlier.
Can we first apply just the minimal platform_data + board file + clock
changes?
That way I can apply those to some immutable tree for everybody to use,
and we cut off the dependency to the driver changes for the rest of the
patches. And then I'm off the hook for the rest of the patches :)
Tony
^ permalink raw reply
* [PATCH 1'/2] ARM: tegra20: Add cpu node
From: Stephen Warren @ 2013-01-11 18:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357910815-31603-1-git-send-email-hdoyu@nvidia.com>
On 01/11/2013 06:26 AM, Hiroshi Doyu wrote:
> Add cpu node for tegra20.
Applied the series to Tegra's for-3.9/soc branch, in anticipation of the
Tegra114 changes being applied to that branch, which depend on these DT
changes.
I made some minor changes to capitalization of the commit messages.
^ permalink raw reply
* [PATCH 10/16] ARM: vexpress: introduce DCSCB support
From: Santosh Shilimkar @ 2013-01-11 18:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357777251-13541-11-git-send-email-nicolas.pitre@linaro.org>
On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
> This adds basic CPU and cluster reset controls on RTSM for the
> A15x4-A7x4 model configuration using the Dual Cluster System
> Configuration Block (DCSCB).
>
> The cache coherency interconnect (CCI) is not handled yet.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/mach-vexpress/Kconfig | 8 ++
> arch/arm/mach-vexpress/Makefile | 1 +
> arch/arm/mach-vexpress/dcscb.c | 160 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 169 insertions(+)
> create mode 100644 arch/arm/mach-vexpress/dcscb.c
>
> diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
> index 99e63f5f99..e55c02562f 100644
> --- a/arch/arm/mach-vexpress/Kconfig
> +++ b/arch/arm/mach-vexpress/Kconfig
> @@ -53,4 +53,12 @@ config ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA
> config ARCH_VEXPRESS_CA9X4
> bool "Versatile Express Cortex-A9x4 tile"
>
> +config ARCH_VEXPRESS_DCSCB
> + bool "Dual Cluster System Control Block (DCSCB) support"
> + depends on BIG_LITTLE
> + help
> + Support for the Dual Cluster System Configuration Block (DCSCB).
> + This is needed to provide CPU and cluster power management
> + on RTSM.
> +
> endmenu
> diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile
> index 80b64971fb..2253644054 100644
> --- a/arch/arm/mach-vexpress/Makefile
> +++ b/arch/arm/mach-vexpress/Makefile
> @@ -6,5 +6,6 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include \
>
> obj-y := v2m.o reset.o
> obj-$(CONFIG_ARCH_VEXPRESS_CA9X4) += ct-ca9x4.o
> +obj-$(CONFIG_ARCH_VEXPRESS_DCSCB) += dcscb.o
> obj-$(CONFIG_SMP) += platsmp.o
> obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
> diff --git a/arch/arm/mach-vexpress/dcscb.c b/arch/arm/mach-vexpress/dcscb.c
> new file mode 100644
> index 0000000000..cccd943cd4
> --- /dev/null
> +++ b/arch/arm/mach-vexpress/dcscb.c
> @@ -0,0 +1,160 @@
> +/*
> + * arch/arm/mach-vexpress/dcscb.c - Dual Cluster System Control Block
> + *
> + * Created by: Nicolas Pitre, May 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.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/io.h>
> +#include <linux/spinlock.h>
> +#include <linux/errno.h>
> +#include <linux/vexpress.h>
> +
> +#include <asm/bL_entry.h>
> +#include <asm/proc-fns.h>
> +#include <asm/cacheflush.h>
> +
> +
> +#define DCSCB_PHYS_BASE 0x60000000
> +
> +#define RST_HOLD0 0x0
> +#define RST_HOLD1 0x4
> +#define SYS_SWRESET 0x8
> +#define RST_STAT0 0xc
> +#define RST_STAT1 0x10
> +#define EAG_CFG_R 0x20
> +#define EAG_CFG_W 0x24
> +#define KFC_CFG_R 0x28
> +#define KFC_CFG_W 0x2c
> +#define DCS_CFG_R 0x30
> +
> +/*
> + * We can't use regular spinlocks. In the switcher case, it is possible
> + * for an outbound CPU to call power_down() after its inbound counterpart
> + * is already live using the same logical CPU number which trips lockdep
> + * debugging.
> + */
> +static arch_spinlock_t dcscb_lock = __ARCH_SPIN_LOCK_UNLOCKED;
> +
> +static void __iomem *dcscb_base;
> +
> +static int dcscb_power_up(unsigned int cpu, unsigned int cluster)
> +{
> + unsigned int rst_hold, cpumask = (1 << cpu);
> +
> + pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
> + if (cpu >= 4 || cluster >= 2)
> + return -EINVAL;
> +
> + /*
> + * Since this is called with IRQs enabled, and no arch_spin_lock_irq
> + * variant exists, we need to disable IRQs manually here.
> + */
> + local_irq_disable();
> + arch_spin_lock(&dcscb_lock);
> +
> + rst_hold = readl_relaxed(dcscb_base + RST_HOLD0 + cluster * 4);
> + if (rst_hold & (1 << 8)) {
> + /* remove cluster reset and add individual CPU's reset */
> + rst_hold &= ~(1 << 8);
> + rst_hold |= 0xf;
> + }
> + rst_hold &= ~(cpumask | (cpumask << 4));
> + writel(rst_hold, dcscb_base + RST_HOLD0 + cluster * 4);
> +
> + arch_spin_unlock(&dcscb_lock);
> + local_irq_enable();
> +
> + return 0;
> +}
> +
> +static void dcscb_power_down(void)
> +{
> + unsigned int mpidr, cpu, cluster, rst_hold, cpumask, last_man;
> +
> + asm ("mrc p15, 0, %0, c0, c0, 5" : "=r" (mpidr));
> + cpu = mpidr & 0xff;
> + cluster = (mpidr >> 8) & 0xff;
> + cpumask = (1 << cpu);
> +
> + pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
> + BUG_ON(cpu >= 4 || cluster >= 2);
> +
> + arch_spin_lock(&dcscb_lock);
> + rst_hold = readl_relaxed(dcscb_base + RST_HOLD0 + cluster * 4);
> + rst_hold |= cpumask;
> + if (((rst_hold | (rst_hold >> 4)) & 0xf) == 0xf)
> + rst_hold |= (1 << 8);
> + writel(rst_hold, dcscb_base + RST_HOLD0 + cluster * 4);
> + arch_spin_unlock(&dcscb_lock);
> + last_man = (rst_hold & (1 << 8));
> +
> + /*
> + * Now let's clean our L1 cache and shut ourself down.
> + * If we're the last CPU in this cluster then clean L2 too.
> + */
> +
Do you wanted to have C bit clear code here ?
> + /*
> + * A15/A7 can hit in the cache with SCTLR.C=0, so we don't need
> + * a preliminary flush here for those CPUs. At least, that's
> + * the theory -- without the extra flush, Linux explodes on
> + * RTSM (maybe not needed anymore, to be investigated)..
> + */
> + flush_cache_louis();
> + cpu_proc_fin();
> +
> + if (!last_man) {
> + flush_cache_louis();
> + } else {
> + flush_cache_all();
> + outer_flush_all();
> + }
> +
> + /* Disable local coherency by clearing the ACTLR "SMP" bit: */
> + asm volatile (
> + "mrc p15, 0, ip, c1, c0, 1 \n\t"
> + "bic ip, ip, #(1 << 6) @ clear SMP bit \n\t"
> + "mcr p15, 0, ip, c1, c0, 1"
> + : : : "ip" );
> +
> + /* Now we are prepared for power-down, do it: */
You need dsb here, right ?
> + wfi();
> +
> + /* Not dead at this point? Let our caller cope. */
> +}
> +
Regards
Santosh
^ permalink raw reply
* [PATCH] ARM: let CPUs not being able to run in ARM mode enter in THUMB mode
From: Russell King - ARM Linux @ 2013-01-11 18:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F0533C.8070802@arm.com>
On Fri, Jan 11, 2013 at 06:00:28PM +0000, Jonathan Austin wrote:
> 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?
Consider this: you run make xconfig (or whatever config tool you
prefer). You're presented with an option, which you want to disable.
It says the possible values for it are 'Y' (n and m are not possible).
So, you start to wonder why this is - why you're even being presented
with an option where it only has one allowable state. That's not an
option, that's an ultimatum!
Now, you ask the config tool for the reason why the symbol is selected.
It gives you a big long list of dependencies - I've seen some of these
which are over 50 lines long. A complex nest of options which are &&
and ||'d together. You've no clue which one(s) result in the symbol
being selected.
So, the presentation of that symbol is nothing more than pure noise
and in some cases just leads to frustration at not being able to achieve
the configuration that's desired.
> (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).
Note: these aren't user visible symbols (note the lack of option text
after 'bool' - that makes them hidden). Instead, these symbols are
to be selected from the CONFIG_CPU_ARM926T etc symbols - so it's a
choice that the developers are making.
We can also add a "help" stanza to both these to describe what they're
representing, or if people prefer a comment before the option.
^ permalink raw reply
* [PATCH 01/16] ARM: b.L: secondary kernel entry code
From: Nicolas Pitre @ 2013-01-11 18:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F048D7.6000904@ti.com>
On Fri, 11 Jan 2013, Santosh Shilimkar wrote:
> On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
> > +ENTRY(bL_entry_point)
> > +
> > + THUMB( adr r12, BSYM(1f) )
> > + THUMB( bx r12 )
> > + THUMB( .thumb )
> > +1:
> > + mrc p15, 0, r0, c0, c0, 5
> > + ubfx r9, r0, #0, #4 @ r9 = cpu
> > + ubfx r10, r0, #8, #4 @ r10 = cluster
> > + mov r3, #BL_CPUS_PER_CLUSTER
> > + mla r4, r3, r10, r9 @ r4 = canonical CPU index
> > + cmp r4, #(BL_CPUS_PER_CLUSTER * BL_NR_CLUSTERS)
> > + blo 2f
> > +
> > + /* We didn't expect this CPU. Try to make it quiet. */
> > +1: wfi
> > + wfe
>
> Why do you need a wfe followed by wif ?
> Just curious.
If the WFI doesn't work because an interrupt is pending then the WFE
might work better. But as I mentioned before, this is not intended to
be used for other purposes than "we're really screwed so at least let's
try to cheaply quieten this CPU" case.
Nicolas
^ permalink raw reply
* [kvmarm] [PATCH v2 2/2] ARM: KVM: Power State Coordination Interface implementation
From: Marc Zyngier @ 2013-01-11 18:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANM98qLRsdFGxU6S5q2EqtCAmK8fOnbGWi8qXBktyuRkSaPAJg@mail.gmail.com>
On 11/01/13 17:48, Christoffer Dall wrote:
> 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.
Sorry, but I do not buy this.
In this particular case, the meaning of the value returned has nothing
to do with the handle_exit convention. We never return to user space,
let alone signaling an error.
Trying to force all the code in this convention makes it actually harder
to review, because this is the exception in the kernel. This is why I
suggest keeping the handle_exit return convention at this exact point,
and not impose it on anything else.
>>
>>> 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.
>>
>
--
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: Dave Martin @ 2013-01-11 18:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F04FEA.1040205@ti.com>
On Fri, Jan 11, 2013 at 11:16:18PM +0530, Santosh Shilimkar wrote:
[...]
> >+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.
Thanks for reading through it. Partly, this was insurance against me
forgetting how the code worked in between writing and posting it...
but this is all quite subtle code, so it felt important to document
it thoroughly.
>
> >+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.
Does kerneldoc not require the comment to be specially formatted?
I haven't played with that, so far.
>
> >+ * __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
The short answer is "maybe not". Some of the barriers can be
eliminated; some can be demoted to DSBs. Others may be required but
unnecessarily duplicated e.g., between bL_head.S and vlock.S.
> any way make it to the main memory, No ?
Yes, but this raises issues about precisely what the architecture
guarantees about memory ordering in these scenarios. The only obvious
thing about that is that it's non-obvious.
Strongly-Ordered memory is not quite the same as having explicit
barriers everywhere.
I need to have a careful think, but it should be possible to optimise
a bit here.
>
> >+
> >+ @ 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.
Same answer... for now
Cheers
---Dave
^ 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