Re: Problems booting exynos5420 with >1 CPU
On Sat, Jun 7, 2014 at 5:19 PM, Russell King - ARM Linux wrote: > On Sat, Jun 07, 2014 at 04:53:34PM -0700, Olof Johansson wrote: >> You do realize that you have absolutely zero leverage over us on this, >> right? Our product is already shipped with kernel code that fixes >> this. > > That is never a justification for forcing /any/ code into the kernel. I'm not looking to force code in, I'm just making it clear that we have limited chance to motivate rework of this since the primary objective of the platform has already been met: We've shipped a product with it. I also don't think it's really in anyone's best interest to have to open up their device, remove write protect, download and build a mainline u-boot and try flashing that onto the system to get it to a state where they can use a mainline kernel. There's too much risk of bricking your hardware if you get it wrong, and you void your warranty. > We've been here before with the iPAQs, where there were all sorts of > horrid hacks that were in the code for that device, and we said no to > it, and we kept it out of the mainline kernel, and stopped those hacks > polluting elsewhere (because people got to know on the whole that if > they used those hacks, it would bar them from mainline participation.) Unfortunately, very few companies actually care about mainline participation today to the point where I don't think they care much any set examples. :( > There's many other instances. Think about it - if we allowed this as > an acceptance criteria, then all that vendors have to do to get their > code into the kernel is change their development cycle: develop > product, ship product, force code into mainline as a done deal not > accepting any review comments back. That is of course not a suitable way of working either. Unfortunately what is mostly happening today is that vendors are doing this: develop product, ship product. The last step never happens. Finding a middle ground where we can pick up some of the platform quirks without making the kernel a big ball of hacks is of course the tricky part. In this specific case, we're not ignoring review feedback, and the comments we've gotten we will absolutely make sure are fixed in the next generation of product (if/when we do another big.LITTLE platform, etc). There's just no room to fix it for the current generation. In hindsight, of course what should have happened is that we should have pushed the vendor to upstream the code sooner, and we're working on making that better in the future too. Since we're talking about upstreaming of platform support, this is something I'm quite passionate about so I'll rant a bit: Looking at the general case more than just this specific instance of Samsung Chromebooks, I think we should in general work hard (where vendors are willing to cooperate) to make sure that we can fully enable support for platforms to the point where they can just run unmodified upstream. Too many of the products today aren't shipping kernels anywhere near what's mainline: It's usually a kernel that is a couple of years old with a few thousand patches on top. It means that bugfixes for the platforms (and much other useful code) doesn't find its way into the kernel, and we have a long (or nonexistent) cycle of feedback due to it. Drivers are in some cases completely broken because nobody actually uses the upstream versions, people post building-but-broken code because they can't actually run and test the patch on any existing hardware with mainline so they just forward-port what they have in the product tree and hope for the best. Etc. I would really like to reach a state where we have several substantial and popular platforms that work well enough with mainline that people can use some of the mainline-near distros to run on the machine. Cubox-i is a great example, even though there are still some pieces left there as well. The new Chromebooks have hardware specs that are quite suitable to use as native development platforms (good CPUs, 4GB ram, and micro-SD card to expand beyond the basic 16GB eMMC), so I think it makes sense to try to enable them and pick up a minimal set of the less than ideal code snippets for that. We'll end up with a better supported and more solid kernel if we can get distros such as Fedora and Debian to ship with these upstream kernels instead of the vendor tree (which is 3.8 based in this case, and won't ever move forward). -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems booting exynos5420 with >1 CPU
On Sat, Jun 07, 2014 at 04:53:34PM -0700, Olof Johansson wrote: > You do realize that you have absolutely zero leverage over us on this, > right? Our product is already shipped with kernel code that fixes > this. That is never a justification for forcing /any/ code into the kernel. We've been here before with the iPAQs, where there were all sorts of horrid hacks that were in the code for that device, and we said no to it, and we kept it out of the mainline kernel, and stopped those hacks polluting elsewhere (because people got to know on the whole that if they used those hacks, it would bar them from mainline participation.) There's many other instances. Think about it - if we allowed this as an acceptance criteria, then all that vendors have to do to get their code into the kernel is change their development cycle: develop product, ship product, force code into mainline as a done deal not accepting any review comments back. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems booting exynos5420 with >1 CPU
Lorenzo,
Since you're emailing from @arm.com, some of this is to the wider
recipient and maybe not directly to you:
On Sat, Jun 7, 2014 at 3:36 PM, Lorenzo Pieralisi
wrote:
> On Sat, Jun 07, 2014 at 09:06:36PM +0100, Nicolas Pitre wrote:
>> On Sat, 7 Jun 2014, Lorenzo Pieralisi wrote:
>>
>> > On Sat, Jun 07, 2014 at 05:10:27PM +0100, Nicolas Pitre wrote:
>> > > On Sat, 7 Jun 2014, Abhilash Kesavan wrote:
>> > >
>> > > > Hi Nicolas,
>> > > >
>> > > > The first man of the incoming cluster enables its snoops via the
>> > > > power_up_setup function. During secondary boot-up, this does not occur
>> > > > for the boot cluster. Hence, I enable the snoops for the boot cluster
>> > > > as a one-time setup from the u-boot prompt. After secondary boot-up
>> > > > there is no modification that I do.
>> > >
>> > > OK that's good.
>> > >
>> > > > Where should this be ideally done ?
>> > >
>> > > If I remember correctly, the CCI can be safely activated only when the
>> > > cache is disabled. So that means the CCI should ideally be turned on
>> > > for the boot cluster (and *only* for the boot CPU) by the bootloader.
>> >
>> > CCI ports are enabled per-cluster, so the boot loader must turn on
>> > CCI for all clusters before the respective CPUs have a chance to
>> > turn on their caches. It is a secure operation, this can be overriden
>> > and probably that's what has been done, wrongly.
>>
>> Careful. By saying "for all clusters" you might be interpreted as
>> saying that the CCI must be turned on even for CPUs that are not powered
>> up.
>
> Right, CCI snoops and DVMs for a cluster must be enabled by the first man
> running in that cluster upon cluster power up with caches off, where the
> first man must be arbitrated if multiple CPUs are racing for enabling CCI.
> This is not an issue on cold boot, it is on resuming from CPUidle.
That's already handled by the MCPM backend on exynos:
/*
* Enable cluster-level coherency, in preparation for turning on the MMU.
*/
static void __naked exynos_pm_power_up_setup(unsigned int affinity_level)
{
asm volatile ("\n"
"cmpr0, #1\n"
"bxne lr\n"
"b cci_enable_port_for_self");
}
>> > True, TC2 on warm-boot reenables CCI, and that's because it runs the
>> > kernel in secure world, and again that's _wrong_.
>>
>> Let me respectfully disagree.
>
> You are welcome =)
>
>> > It must be done in firmware, and I am totally against any attempt at
>> > papering over what looks like a messed up firmware implementation with
>> > a bunch of hacks in the kernel, because that's what the patch below is
>> > (soft restarting a CPU to enable CCI ? and adding generic code for that ?
>> > what's next ?)
>>
>> Are you promoting for the removal of drivers/bus/arm-cci.c ?
>
> I really do not want to go there, but I might (apart from CCI PMUs).
>
>> You do realize that the fundamental raison d'?tre for MCPM is actually
>> to manage the race free enabling of the cache and CCI ?
>
> Yes I do and I was willing to help implement it for TC2 to provide people
> with an example on how to do it _properly_ (in secure world though, and that
> was a mistake IMHO). If what we get is a workaround for every platform
> going upstream, we are in a bind, seriously.
Real life is messy. Bugs happen. Having a stance that the kernel has
to be a puritan implementation that can be done in a vacuum where bugs
in hardware and firmware don't exist (and are fixable in the right
place every single case) is not realistic. Linux is a useless piece of
software to us if that is the case.
I've seen this from several other ARM developers in the past. It's
almost like they were a couple of degrees removed from actually
working on shipping real products and dealing with real users.
> Whatever the outcome of this thread, a booting protocol update for CCI
> is in order, even if we have to debate it for 6 months or more to get
> an agreement.
Ding ding ding. Current documentation is in some very complex C
frameworks (MCPM), and some device tree bindings that obviously don't
cover this. Real documentation is obviously needed, but more than that
(see below).
I'd actually argue that you don't have a leg to stand on in your
complaints at all because:
1) There's no documentation of the requirements
2) The only existing golden platform (TC2) manages CCI similar to how
exynos does.
>> > I understand there is an issue and lots at stake here, but I do not want
>> > the
>> > patch below to be merged in the kernel, I am sorry, it is a tad too much.
>>
>> Lorenzo: Don't get me wrong. The Chromebooks, and possibly to some
>> extent some people at Samsung, were simply too confident in their
>> ability to create absolutely bug-free firmware code to the point of not
>> making its update easy in the field. This is completely outrageous in
>> my point of view.
I would like to clarify that firmware is updateable today. But
Chromebooks are in many ways vertically integrated products, so if a
problem is f
Re: Problems booting exynos5420 with >1 CPU
On Sat, Jun 07, 2014 at 09:06:36PM +0100, Nicolas Pitre wrote: > On Sat, 7 Jun 2014, Lorenzo Pieralisi wrote: > > > On Sat, Jun 07, 2014 at 05:10:27PM +0100, Nicolas Pitre wrote: > > > On Sat, 7 Jun 2014, Abhilash Kesavan wrote: > > > > > > > Hi Nicolas, > > > > > > > > The first man of the incoming cluster enables its snoops via the > > > > power_up_setup function. During secondary boot-up, this does not occur > > > > for the boot cluster. Hence, I enable the snoops for the boot cluster > > > > as a one-time setup from the u-boot prompt. After secondary boot-up > > > > there is no modification that I do. > > > > > > OK that's good. > > > > > > > Where should this be ideally done ? > > > > > > If I remember correctly, the CCI can be safely activated only when the > > > cache is disabled. So that means the CCI should ideally be turned on > > > for the boot cluster (and *only* for the boot CPU) by the bootloader. > > > > CCI ports are enabled per-cluster, so the boot loader must turn on > > CCI for all clusters before the respective CPUs have a chance to > > turn on their caches. It is a secure operation, this can be overriden > > and probably that's what has been done, wrongly. > > Careful. By saying "for all clusters" you might be interpreted as > saying that the CCI must be turned on even for CPUs that are not powered > up. Right, CCI snoops and DVMs for a cluster must be enabled by the first man running in that cluster upon cluster power up with caches off, where the first man must be arbitrated if multiple CPUs are racing for enabling CCI. This is not an issue on cold boot, it is on resuming from CPUidle. > > True, TC2 on warm-boot reenables CCI, and that's because it runs the > > kernel in secure world, and again that's _wrong_. > > Let me respectfully disagree. You are welcome =) > > It must be done in firmware, and I am totally against any attempt at > > papering over what looks like a messed up firmware implementation with > > a bunch of hacks in the kernel, because that's what the patch below is > > (soft restarting a CPU to enable CCI ? and adding generic code for that ? > > what's next ?) > > Are you promoting for the removal of drivers/bus/arm-cci.c ? I really do not want to go there, but I might (apart from CCI PMUs). > You do realize that the fundamental raison d'?tre for MCPM is actually > to manage the race free enabling of the cache and CCI ? Yes I do and I was willing to help implement it for TC2 to provide people with an example on how to do it _properly_ (in secure world though, and that was a mistake IMHO). If what we get is a workaround for every platform going upstream, we are in a bind, seriously. Whatever the outcome of this thread, a booting protocol update for CCI is in order, even if we have to debate it for 6 months or more to get an agreement. > > I understand there is an issue and lots at stake here, but I do not want the > > patch below to be merged in the kernel, I am sorry, it is a tad too much. > > Lorenzo: Don't get me wrong. The Chromebooks, and possibly to some > extent some people at Samsung, were simply too confident in their > ability to create absolutely bug-free firmware code to the point of not > making its update easy in the field. This is completely outrageous in > my point of view. Yet one of the reactions was to call upstream kernel > people as purists because the kernel is so much easier to modify in > order to cover their mess and yet that might not be accepted. Like I > said I won't stop shaming them publicly for their own "incompetence" > just yet (no pun intended), but being excessively purist does not > benefit anyone either, and for that they have a point. I do not think they do. The kernel should not become a place where firmware bugs are fixed, if you refuse to fix the bug and this code does not get upstream I am pretty sure next time more attention will be paid. Booting, powering up and down cores are standard procedures, why can't we share the same code for all v7 platforms ? Why ? And we are not talking about a race condition hit every 10 gazillions cycles here. > *HOWEVER* I have no choice but to say that many people at ARM, including > a couple individuals for whom I nevertheless have a lot of admiration, > also have an incredible faith in their ability to convince themselves, > and then turn around to preach to the world, that *more firmware* is > going to be so much purer and solve so many more problems than it > creates and become such a magical success that we should immediately > dedicate our soul to the cause with no hint of a doubt. > > I'm sorry to rain on your parade, but I don't believe in this one iota. > > Let me repeat the MCPM story again: it took 3 people, including 2 from > ARM, over *six* months to get everything right and stable on TC2. I > think you also contributed to that effort as well. Subsequent MCPM > backend contributions (yes, just the backend and not the core code)
Re: Problems booting exynos5420 with >1 CPU
On Sat, 7 Jun 2014, Lorenzo Pieralisi wrote: > On Sat, Jun 07, 2014 at 05:10:27PM +0100, Nicolas Pitre wrote: > > On Sat, 7 Jun 2014, Abhilash Kesavan wrote: > > > > > Hi Nicolas, > > > > > > The first man of the incoming cluster enables its snoops via the > > > power_up_setup function. During secondary boot-up, this does not occur > > > for the boot cluster. Hence, I enable the snoops for the boot cluster > > > as a one-time setup from the u-boot prompt. After secondary boot-up > > > there is no modification that I do. > > > > OK that's good. > > > > > Where should this be ideally done ? > > > > If I remember correctly, the CCI can be safely activated only when the > > cache is disabled. So that means the CCI should ideally be turned on > > for the boot cluster (and *only* for the boot CPU) by the bootloader. > > CCI ports are enabled per-cluster, so the boot loader must turn on > CCI for all clusters before the respective CPUs have a chance to > turn on their caches. It is a secure operation, this can be overriden > and probably that's what has been done, wrongly. Careful. By saying "for all clusters" you might be interpreted as saying that the CCI must be turned on even for CPUs that are not powered up. > True, TC2 on warm-boot reenables CCI, and that's because it runs the > kernel in secure world, and again that's _wrong_. Let me respectfully disagree. > It must be done in firmware, and I am totally against any attempt at > papering over what looks like a messed up firmware implementation with > a bunch of hacks in the kernel, because that's what the patch below is > (soft restarting a CPU to enable CCI ? and adding generic code for that ? > what's next ?) Are you promoting for the removal of drivers/bus/arm-cci.c ? You do realize that the fundamental raison d'être for MCPM is actually to manage the race free enabling of the cache and CCI ? > I understand there is an issue and lots at stake here, but I do not want the > patch below to be merged in the kernel, I am sorry, it is a tad too much. Lorenzo: Don't get me wrong. The Chromebooks, and possibly to some extent some people at Samsung, were simply too confident in their ability to create absolutely bug-free firmware code to the point of not making its update easy in the field. This is completely outrageous in my point of view. Yet one of the reactions was to call upstream kernel people as purists because the kernel is so much easier to modify in order to cover their mess and yet that might not be accepted. Like I said I won't stop shaming them publicly for their own "incompetence" just yet (no pun intended), but being excessively purist does not benefit anyone either, and for that they have a point. *HOWEVER* I have no choice but to say that many people at ARM, including a couple individuals for whom I nevertheless have a lot of admiration, also have an incredible faith in their ability to convince themselves, and then turn around to preach to the world, that *more firmware* is going to be so much purer and solve so many more problems than it creates and become such a magical success that we should immediately dedicate our soul to the cause with no hint of a doubt. I'm sorry to rain on your parade, but I don't believe in this one iota. Let me repeat the MCPM story again: it took 3 people, including 2 from ARM, over *six* months to get everything right and stable on TC2. I think you also contributed to that effort as well. Subsequent MCPM backend contributions (yes, just the backend and not the core code) took at least *five* rounds of reviews in one case, and after *seven* rounds in another case it is still not right, despite the publicly available TC2 implementation to serve as a reference. I'm sure each time a new patch set was posted, their authors honestly believed their code was correct. Otherwise why would they post buggy code? Now you are telling me that they should have put that code into firmware instead? Can you realize what a catastrophe this would have been? Are you _seriously_ believing that they would be up to their 5th firmware revision by now? And that updating their firmware six months after product launch would be as easy as updating the kernel? Software ALWAYS has bugs, whether it is user apps, the kernel, firmware or boot ROM. The bigger one of those parts is, the more bugs it will have. And the cost to vendors for fixing those bugs grow exponentially down each level. For proof, we're now considering possible workarounds in the kernel to sidestep the difficulty with updating the firmware on a Chromebook. Yet you're saying that firmware should grow code with the same complexity as the MCPM core, plus a machine specific backend that experience has proven multiple times is evidently hard to get right, into firmware because running Linux in secure mode is wrong? If so we don't live in the same world indeed. The day I see a firmware architecture that allows for
Re: [PATCH] ARM: EXYNOS: mcpm: Don't rely on firmware's secondary_cpu_start
On Fri, Jun 06, 2014 at 10:43:05PM +0100, Doug Anderson wrote:
> On exynos mcpm systems the firmware is hardcoded to jump to an address
> in SRAM (0x02073000) when secondary CPUs come up. By default the
> firmware puts a bunch of code at that location. That code expects the
> kernel to fill in a few slots with addresses that it uses to jump back
> to the kernel's entry point for secondary CPUs.
>
> Originally (on prerelease hardware) this firmware code contained a
> bunch of workarounds to deal with boot ROM bugs. However on all
> shipped hardware we simply use this code to redirect to a kernel
> function for bringing up the CPUs.
>
> Let's stop relying on the code provided by the bootloader and just
> plumb in our own (simple) code jump to the kernel. This has the nice
> benefit of fixing problems due to the fact that older bootloaders
> (like the one shipped on the Samsung Chromebook 2) might have put
> slightly different code into this location.
>
> Once suspend/resume is implemented for systems using exynos-mcpm we'll
> need to make sure we reinstall our fixed up code after resume. ...but
> that's not anything new since IRAM (and thus the address of the
> mcpm_entry_point) is lost across suspend/resume anyway.
Can I ask you please what the firmware does for the boot (primary) cpu
on cold-boot and warm-boot (resume from suspend) ?
Does it jump to a specific (hardcoded) location ?
Is the primary CPU (MPIDR) hardcoded in firmware so that on both
cold and warm-boot firmware sees a specific MPIDR as "special" ?
I am asking to check if on this platform CPUidle (where the notion of
primary CPU disappears) has a chance to run properly.
Probably CPUidle won't attain idle states where IRAM content is lost, but I
am still worried about the primary vs secondaries firmware boot behaviour.
What happens on reboot from suspend to RAM (or to put it differently,
what does secure firmware do on reboot from suspend to RAM - in
particular how is the "jump" address to bootloader/kernel set ?)
Thank you very much.
Lorenzo
>
> Signed-off-by: Doug Anderson
> ---
> arch/arm/mach-exynos/mcpm-exynos.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c
> b/arch/arm/mach-exynos/mcpm-exynos.c
> index 0498d0b..3a7fad0 100644
> --- a/arch/arm/mach-exynos/mcpm-exynos.c
> +++ b/arch/arm/mach-exynos/mcpm-exynos.c
> @@ -343,11 +343,13 @@ static int __init exynos_mcpm_init(void)
> pr_info("Exynos MCPM support installed\n");
>
> /*
> - * Future entries into the kernel can now go
> - * through the cluster entry vectors.
> + * U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr
> + * as part of secondary_cpu_start(). Let's redirect it to the
> + * mcpm_entry_point().
>*/
> - __raw_writel(virt_to_phys(mcpm_entry_point),
> - ns_sram_base_addr + MCPM_BOOT_ADDR_OFFSET);
> + __raw_writel(0xe59f, ns_sram_base_addr); /* ldr r0, [pc, #0] */
> + __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */
> + __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8);
>
> iounmap(ns_sram_base_addr);
>
> --
> 2.0.0.526.g5318336
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems booting exynos5420 with >1 CPU
On Sat, Jun 07, 2014 at 05:10:27PM +0100, Nicolas Pitre wrote:
> On Sat, 7 Jun 2014, Abhilash Kesavan wrote:
>
> > Hi Nicolas,
> >
> > The first man of the incoming cluster enables its snoops via the
> > power_up_setup function. During secondary boot-up, this does not occur
> > for the boot cluster. Hence, I enable the snoops for the boot cluster
> > as a one-time setup from the u-boot prompt. After secondary boot-up
> > there is no modification that I do.
>
> OK that's good.
>
> > Where should this be ideally done ?
>
> If I remember correctly, the CCI can be safely activated only when the
> cache is disabled. So that means the CCI should ideally be turned on
> for the boot cluster (and *only* for the boot CPU) by the bootloader.
CCI ports are enabled per-cluster, so the boot loader must turn on
CCI for all clusters before the respective CPUs have a chance to
turn on their caches. It is a secure operation, this can be overriden
and probably that's what has been done, wrongly.
True, TC2 on warm-boot reenables CCI, and that's because it runs the
kernel in secure world, and again that's _wrong_.
It must be done in firmware, and I am totally against any attempt at
papering over what looks like a messed up firmware implementation with
a bunch of hacks in the kernel, because that's what the patch below is
(soft restarting a CPU to enable CCI ? and adding generic code for that ?
what's next ?)
I understand there is an issue and lots at stake here, but I do not want the
patch below to be merged in the kernel, I am sorry, it is a tad too much.
Lorenzo
>
> Now... If you _really_ prefer to do it from the kernel to avoid
> difficulties with bootloader updates, then it should be possible to do
> it from the kernel by temporarily turning the cache off. This is not a
> small thing but the MCPM infrastructure can be leveraged. Here's what I
> tried on a TC2 which might just work for you as well:
>
> diff --git a/arch/arm/common/mcpm_entry.c b/arch/arm/common/mcpm_entry.c
> index 86fd60fefb..1cc49de308 100644
> --- a/arch/arm/common/mcpm_entry.c
> +++ b/arch/arm/common/mcpm_entry.c
> @@ -12,11 +12,13 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> #include
> #include
> +#include
>
> extern unsigned long
> mcpm_entry_vectors[MAX_NR_CLUSTERS][MAX_CPUS_PER_CLUSTER];
>
> @@ -146,6 +148,44 @@ int mcpm_cpu_powered_up(void)
> return 0;
> }
>
> +static int go_down(unsigned long _arg)
> +{
> + void (*cache_disable)(void) = (void *)_arg;
> + unsigned int mpidr = read_cpuid_mpidr();
> + unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> + unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> + phys_reset_t phys_reset;
> +
> + mcpm_set_entry_vector(cpu, cluster, cpu_resume);
> + setup_mm_for_reboot();
> +
> + __mcpm_cpu_going_down(cpu, cluster);
> + BUG_ON(!__mcpm_outbound_enter_critical(cpu, cluster));
> + cache_disable();
> + __mcpm_outbound_leave_critical(cluster, CLUSTER_DOWN);
> + __mcpm_cpu_down(cpu, cluster);
> +
> + phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
> + phys_reset(virt_to_phys(mcpm_entry_point));
> + BUG();
> +}
> +
> +int mcpm_loopback(void (*cache_disable)(void))
> +{
> + int ret;
> +
> + local_irq_disable();
> + local_fiq_disable();
> + ret = cpu_pm_enter();
> + if (!ret) {
> + ret = cpu_suspend((unsigned long)cache_disable, go_down);
> + cpu_pm_exit();
> + }
> + local_fiq_enable();
> + local_irq_enable();
> + return ret;
> +}
> +
> struct sync_struct mcpm_sync;
>
> /*
> diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c
> index 29e7785a54..abecdd734f 100644
> --- a/arch/arm/mach-vexpress/tc2_pm.c
> +++ b/arch/arm/mach-vexpress/tc2_pm.c
> @@ -323,6 +323,26 @@ static void __naked tc2_pm_power_up_setup(unsigned int
> affinity_level)
> "b cci_enable_port_for_self ");
> }
>
> +int mcpm_loopback(void (*cache_disable)(void));
> +
> +static void tc2_cache_down(void)
> +{
> + pr_warn("TC2: disabling cache\n");
> + if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A15) {
> + /*
> + * On the Cortex-A15 we need to disable
> + * L2 prefetching before flushing the cache.
> + */
> + asm volatile(
> + "mcrp15, 1, %0, c15, c0, 3 \n\t"
> + "isb\n\t"
> + "dsb"
> + : : "r" (0x400) );
> + }
> + v7_exit_coherency_flush(all);
> + cci_disable_port_by_cpu(read_cpuid_mpidr());
> +}
> +
> static int __init tc2_pm_init(void)
> {
> int ret, irq;
> @@ -370,6 +390,7 @@ static int __init tc2_pm_init(void)
> ret = mcpm_platform_register(&tc2_pm_power_ops);
> if (!ret) {
> mcpm_sync_init(tc2_pm_power_up_setup);
> + BUG_ON(mcpm_loopback(tc2_cache_down) != 0);
> pr_info(
Re: Problems booting exynos5420 with >1 CPU
On Sat, 7 Jun 2014, Abhilash Kesavan wrote:
> Hi Nicolas,
>
> The first man of the incoming cluster enables its snoops via the
> power_up_setup function. During secondary boot-up, this does not occur
> for the boot cluster. Hence, I enable the snoops for the boot cluster
> as a one-time setup from the u-boot prompt. After secondary boot-up
> there is no modification that I do.
OK that's good.
> Where should this be ideally done ?
If I remember correctly, the CCI can be safely activated only when the
cache is disabled. So that means the CCI should ideally be turned on
for the boot cluster (and *only* for the boot CPU) by the bootloader.
Now... If you _really_ prefer to do it from the kernel to avoid
difficulties with bootloader updates, then it should be possible to do
it from the kernel by temporarily turning the cache off. This is not a
small thing but the MCPM infrastructure can be leveraged. Here's what I
tried on a TC2 which might just work for you as well:
diff --git a/arch/arm/common/mcpm_entry.c b/arch/arm/common/mcpm_entry.c
index 86fd60fefb..1cc49de308 100644
--- a/arch/arm/common/mcpm_entry.c
+++ b/arch/arm/common/mcpm_entry.c
@@ -12,11 +12,13 @@
#include
#include
#include
+#include
#include
#include
#include
#include
+#include
extern unsigned long mcpm_entry_vectors[MAX_NR_CLUSTERS][MAX_CPUS_PER_CLUSTER];
@@ -146,6 +148,44 @@ int mcpm_cpu_powered_up(void)
return 0;
}
+static int go_down(unsigned long _arg)
+{
+ void (*cache_disable)(void) = (void *)_arg;
+ unsigned int mpidr = read_cpuid_mpidr();
+ unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+ unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+ phys_reset_t phys_reset;
+
+ mcpm_set_entry_vector(cpu, cluster, cpu_resume);
+ setup_mm_for_reboot();
+
+ __mcpm_cpu_going_down(cpu, cluster);
+ BUG_ON(!__mcpm_outbound_enter_critical(cpu, cluster));
+ cache_disable();
+ __mcpm_outbound_leave_critical(cluster, CLUSTER_DOWN);
+ __mcpm_cpu_down(cpu, cluster);
+
+ phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+ phys_reset(virt_to_phys(mcpm_entry_point));
+ BUG();
+}
+
+int mcpm_loopback(void (*cache_disable)(void))
+{
+ int ret;
+
+ local_irq_disable();
+ local_fiq_disable();
+ ret = cpu_pm_enter();
+ if (!ret) {
+ ret = cpu_suspend((unsigned long)cache_disable, go_down);
+ cpu_pm_exit();
+ }
+ local_fiq_enable();
+ local_irq_enable();
+ return ret;
+}
+
struct sync_struct mcpm_sync;
/*
diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c
index 29e7785a54..abecdd734f 100644
--- a/arch/arm/mach-vexpress/tc2_pm.c
+++ b/arch/arm/mach-vexpress/tc2_pm.c
@@ -323,6 +323,26 @@ static void __naked tc2_pm_power_up_setup(unsigned int
affinity_level)
" b cci_enable_port_for_self ");
}
+int mcpm_loopback(void (*cache_disable)(void));
+
+static void tc2_cache_down(void)
+{
+ pr_warn("TC2: disabling cache\n");
+ if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A15) {
+ /*
+* On the Cortex-A15 we need to disable
+* L2 prefetching before flushing the cache.
+*/
+ asm volatile(
+ "mcrp15, 1, %0, c15, c0, 3 \n\t"
+ "isb\n\t"
+ "dsb"
+ : : "r" (0x400) );
+ }
+ v7_exit_coherency_flush(all);
+ cci_disable_port_by_cpu(read_cpuid_mpidr());
+}
+
static int __init tc2_pm_init(void)
{
int ret, irq;
@@ -370,6 +390,7 @@ static int __init tc2_pm_init(void)
ret = mcpm_platform_register(&tc2_pm_power_ops);
if (!ret) {
mcpm_sync_init(tc2_pm_power_up_setup);
+ BUG_ON(mcpm_loopback(tc2_cache_down) != 0);
pr_info("TC2 power management initialized\n");
}
return ret;
Of course it is not because I'm helping you sidestepping firmware
problems that I'll stop shaming those who came up with that firmware
design. ;-)
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] devicetree: Add generic IOMMU device tree bindings
On Saturday 07 June 2014 00:45:45 Thierry Reding wrote: > This is somewhat off-topic, but given the various concepts discussed in > this thread I'm beginning to wonder how they will be implemented. I think it's good you raised the question. > The > current implementation hooks the IOMMU API into the DMA mapping API, and > the way this is done is by setting a single IOMMU (or rather a set of > IOMMU operations) globally per bus type. I hadn't realized that we have a per-bus iommu_ops pointer. I agree this will become a limitation as soon as we have a soc with two different IOMMUs that have platform devices attached, and it has to be moved into the device or a structure related to that. If that turns out controversial, we can probably have a set of pseudo iommu ops that just call into dev->archdata->iommu_ops->function() for ARM. > There are two issues that I can see with that: one is that we can't > support multiple IOMMUs in the system at all, and the other is that > there is no context associated with the IOMMU ops, and therefore there > is no way to differentiate between two instances of the same IOMMU. A > few drivers use global variables to keep track of context information > but that won't work with multiple instances, unless they keep a global > list of all instances and then require explicit lookup in each of the > IOMMU operations. That sounds more like a workaround rather than a > proper solution to me. Supporting multiple iommus that share one iommu driver should work without such hacks, as you can put the per-device information into dev->device_dma_parameters (this works only for very simple IOMMUs) or dev->archdata->iommu (we may want to generalize that, I think someone just posted patches for it). > Discussion in this thread indicates that both of the above will be > required at some point. Have I completely missed something or will we > have to rework (parts of) the IOMMU API to make this work? > > One other thing that I have some difficulty understanding is how we can > support things like process isolation using the current IOMMU API. Since > a device will be statically assigned to one IOMMU domain at probe time > there is no way we can change the address space upon a context switch. We have just introduced a way to parse dma-ranges in of_dma_configure(). The only way I see this done for platform devices is to do the IOMMU configuration in the same place: if an iommus property is found there, we call out to the iommu driver that matches the respective iommu device and let it configure the master device. The device already has multiple properties related to iommus: 'struct device_dma_parameters', 'archdata', 'iommu_group', and pdev_archdata for platform devices. This should be enough to set up the default iommu dma_map_ops so we can have non-isolated DMA using dma_map_* and dma_alloc_coherent. I haven't given much thought to devices that want to use the IOMMU API directly so they can have multiple domains rather than rely on the dma-mapping abstraction. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: dts: Add missing clock/pin-control entries for I2S0 on Exynos4
I2S driver uses 3 clocks under different conditions. Added two
missing clocks.
Additionally updated pin-control property for this node.
Signed-off-by: Tushar Behera
---
Based on next-20140606.
Tested on Exynos4210-based Origen boards with private patches for
codec/machine driver.
arch/arm/boot/dts/exynos4.dtsi |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index fbaf426..10bb081 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -55,11 +55,15 @@
i2s0: i2s@0383 {
compatible = "samsung,s5pv210-i2s";
reg = <0x0383 0x100>;
- clocks = <&clock_audss EXYNOS_I2S_BUS>;
- clock-names = "iis";
+ clocks = <&clock_audss EXYNOS_I2S_BUS>,
+ <&clock_audss EXYNOS_I2S_BUS>,
+ <&clock_audss EXYNOS_SCLK_I2S>;
+ clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
dmas = <&pdma0 12>, <&pdma0 11>, <&pdma0 10>;
dma-names = "tx", "rx", "tx-sec";
samsung,idma-addr = <0x0300>;
+ pinctrl-0 = <&i2s0_bus>;
+ pinctrl-names = "default";
status = "disabled";
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
