[PATCH v2 0/6] Enable L2 cache support on Exynos4210/4x12 SoCs
This series intends to add support for L2 cache on Exynos4 SoCs on boards
running under secure firmware, which requires certain initialization steps
to be done with help of firmware, as selected registers are writable only
from secure mode.
First four patches extend existing support for secure write in L2C driver
to account for design of secure firmware running on Exynos. Namely:
1) direct read access to certain registers is needed on Exynos, because
secure firmware calls set several registers at once,
2) not all boards are running secure firmware, so .write_sec callback
needs to be installed in Exynos firmware ops initialization code,
3) write access to {DATA,TAG}_LATENCY_CTRL registers fron non-secure world
is not allowed and so must use l2c_write_sec as well,
4) on certain boards, default value of prefetch register is incorrect
and must be overridden at L2C initialization.
Patches 1-3 might affect other platforms using .write_sec callback, so
I'd like to kindly ask any interested people for testing.
Further two patches add impelmentation of .write_sec for Exynos secure
firmware and necessary DT nodes to enable L2 cache.
Tested on Exynos4210-based Universal C210 and Trats (both without secure
firmware) and Exynos4412-based TRATS2 and ODROID-U3 boards (both with secure
firmware).
Changes since v1:
(https://www.mail-archive.com/linux-omap@vger.kernel.org/msg106323.html)
- rebased onto for-next branch of linux-samsung tree,
- changed argument order of outer_cache.write_sec() callback to match
l2c_write_sec() function in cache-l2x0.c,
- added support of overriding of prefetch settings to work around incorrect
default settings on certain Exynos4x12-based boards,
- added call to firmware to invalidate whole L2 cache before setting enable
bit in L2C control register (required by Exynos secure firmware).
Tomasz Figa (6):
ARM: mm: cache-l2x0: Add base address argument to write_sec callback
ARM: Get outer cache .write_sec callback from mach_desc only if not
NULL
ARM: mm: cache-l2x0: Use l2c_write_sec() for LATENCY_CTRL registers
ARM: mm: l2x0: Add support for overriding prefetch settings
ARM: EXYNOS: Add .write_sec outer cache callback for L2C-310
ARM: dts: exynos4: Add nodes for L2 cache controller
Documentation/devicetree/bindings/arm/l2cc.txt | 10
arch/arm/boot/dts/exynos4210.dtsi | 9
arch/arm/boot/dts/exynos4x12.dtsi | 14 ++
arch/arm/include/asm/mach/arch.h | 3 +-
arch/arm/include/asm/outercache.h | 2 +-
arch/arm/kernel/irq.c | 3 +-
arch/arm/mach-exynos/firmware.c| 63 +
arch/arm/mach-highbank/highbank.c | 3 +-
arch/arm/mach-omap2/omap4-common.c | 3 +-
arch/arm/mach-ux500/cache-l2x0.c | 3 +-
arch/arm/mm/cache-l2x0.c | 64 ++
11 files changed, 162 insertions(+), 15 deletions(-)
--
1.9.3
--
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 0/6] Enable L2 cache support on Exynos4210/4x12 SoCs
On Wed, Jun 25, 2014 at 03:37:25PM +0200, Tomasz Figa wrote: This series intends to add support for L2 cache on Exynos4 SoCs on boards running under secure firmware, which requires certain initialization steps to be done with help of firmware, as selected registers are writable only from secure mode. What I said in my message on June 12th applies to this series. I'm not having the virtual address exposed via the write_sec call. Yes, you need to read other registers in order to use your secure firmware implementation. Let's fix that by providing a better write_sec interface so you don't have to read back these registers, rather than working around this short-coming. That's exactly what I meant when I talked on June 12th about turning cache-l2x0.c back into a pile of crap. You're working around problems rather than fixing the underlying issue, as seems to be standard platform maintainer behaviour when things like core ARM code is concerned. This is why things devolve over time into piles of crap, because platforms just hack around problems rather than fixing the root cause of the problem. So... I'm NAKing the entire series. -- 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: [PATCH v2 0/6] Enable L2 cache support on Exynos4210/4x12 SoCs
On 25.06.2014 15:50, Russell King - ARM Linux wrote: On Wed, Jun 25, 2014 at 03:37:25PM +0200, Tomasz Figa wrote: This series intends to add support for L2 cache on Exynos4 SoCs on boards running under secure firmware, which requires certain initialization steps to be done with help of firmware, as selected registers are writable only from secure mode. What I said in my message on June 12th applies to this series. I'm not having the virtual address exposed via the write_sec call. Yes, you need to read other registers in order to use your secure firmware implementation. Let's fix that by providing a better write_sec interface so you don't have to read back these registers, rather than working around this short-coming. Do you have anything in particular in mind? I would be glad to implement it and send patches. That's exactly what I meant when I talked on June 12th about turning cache-l2x0.c back into a pile of crap. You're working around problems rather than fixing the underlying issue, as seems to be standard platform maintainer behaviour when things like core ARM code is concerned. This is why things devolve over time into piles of crap, because platforms just hack around problems rather than fixing the root cause of the problem. I'm not sure what part of my patches exactly is turning cache-l2x0.c into a pile of crap. On the contrary, I believe that working around the firmware brokenness on platform level, while keeping the core code simple does the opposite. However, I'll be happy to rework my series if you have some more specific suggestions. So... I'm NAKing the entire series. Your opinion is always appreciated, thanks. Best regards, Tomasz -- 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 0/6] Enable L2 cache support on Exynos4210/4x12 SoCs
On Wed, Jun 25, 2014 at 04:13:16PM +0200, Tomasz Figa wrote: On 25.06.2014 15:50, Russell King - ARM Linux wrote: On Wed, Jun 25, 2014 at 03:37:25PM +0200, Tomasz Figa wrote: This series intends to add support for L2 cache on Exynos4 SoCs on boards running under secure firmware, which requires certain initialization steps to be done with help of firmware, as selected registers are writable only from secure mode. What I said in my message on June 12th applies to this series. I'm not having the virtual address exposed via the write_sec call. Yes, you need to read other registers in order to use your secure firmware implementation. Let's fix that by providing a better write_sec interface so you don't have to read back these registers, rather than working around this short-coming. Do you have anything in particular in mind? I would be glad to implement it and send patches. As I've already said, you are not the only ones who need fuller information to make your secure monitor calls. So, what that implies is that rather than the interface being please write register X with value V, and then having platforms work-around that by reading various registers, we need a more flexible interface which passes the desired state. That's exactly what I meant when I talked on June 12th about turning cache-l2x0.c back into a pile of crap. You're working around problems rather than fixing the underlying issue, as seems to be standard platform maintainer behaviour when things like core ARM code is concerned. This is why things devolve over time into piles of crap, because platforms just hack around problems rather than fixing the root cause of the problem. I'm not sure what part of my patches exactly is turning cache-l2x0.c into a pile of crap. On the contrary, I believe that working around the firmware brokenness on platform level, while keeping the core code simple does the opposite. What you're doing is making the future maintanence of cache-l2x0.c harder by breaking the modularity of it. By exposing the virtual address of the registers, reading the registers and getting your secure monitor to write them back, you're making assumptions about the behaviours inside cache-l2x0.c - while this may seem to be a no-op think about what happens a few years down the line when someone needs to change how this stuff operates, and you've long since moved on to new pastures and aren't around to answer questions on the exynos implementation. Compare that to modifying the implementation to give the secure monitors what they want - the desired state of the secure registers when enabling and resuming the L2 cache. The API becomes much clearer, and maintainable, because - from the core persective, there isn't a load of platforms doing weird crap with an API which doesn't really fit what they're trying to do. So, if we accept your patches as is and let that approach continue, then years down the line, cleaning up the crap that you've deposited becomes much harder, and we end up either having to break Exynos declaring it as a SoC we just don't give a damn about it, or keep the old interface. That is bad news which ever way you look at it. Quite simply, if a job is worth doing, then it's worth doing well and properly. The reason we have l2c_write_sec() right now is that when I sorted out the existing shitpile that platform maintainers had turned that code into over the years, it was the interface which suited the current implementations. As the secure APIs are typically confidential, there is no way to consider what other alternatives are out there, so this is something that is going to have to remain flexible - in other words, it needs to remain long term maintainable, so that it can change. So, working around it's short-comings in platform code is totally the wrong thing to do. -- 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: [PATCH v2 0/6] Enable L2 cache support on Exynos4210/4x12 SoCs
On 25.06.2014 16:37, Russell King - ARM Linux wrote: On Wed, Jun 25, 2014 at 04:13:16PM +0200, Tomasz Figa wrote: On 25.06.2014 15:50, Russell King - ARM Linux wrote: On Wed, Jun 25, 2014 at 03:37:25PM +0200, Tomasz Figa wrote: This series intends to add support for L2 cache on Exynos4 SoCs on boards running under secure firmware, which requires certain initialization steps to be done with help of firmware, as selected registers are writable only from secure mode. What I said in my message on June 12th applies to this series. I'm not having the virtual address exposed via the write_sec call. Yes, you need to read other registers in order to use your secure firmware implementation. Let's fix that by providing a better write_sec interface so you don't have to read back these registers, rather than working around this short-coming. Do you have anything in particular in mind? I would be glad to implement it and send patches. As I've already said, you are not the only ones who need fuller information to make your secure monitor calls. So, what that implies is that rather than the interface being please write register X with value V, and then having platforms work-around that by reading various registers, we need a more flexible interface which passes the desired state. So it's still not clear to me how this should be done correctly. One thing that comes to my mind is precomputing register values to some kind of structure, then calling some kind of magical platform-specific .enable() or .configure() callback, which takes the structure and, in one shot, configures the L2C according to firmware requirements. Then the generic code would read back those values to verify the final configuration (as it does right now) and rest of the operation would be identical. Is this something you would deem acceptable? Best regards, Tomasz -- 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
