* [PATCH v2 4/7] clk: Add simple gated clock
From: Grant Likely @ 2011-09-25 4:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316730422-20027-5-git-send-email-mturquette@ti.com>
On Thu, Sep 22, 2011 at 03:26:59PM -0700, Mike Turquette wrote:
> From: Jeremy Kerr <jeremy.kerr@canonical.com>
>
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> Signed-off-by: Mike Turquette <mturquette@ti.com>
> ---
> Changes since v1:
> Add copyright header
> Fold in Jamie's patch for set-to-disable clks
> Use BIT macro instead of shift
>
> drivers/clk/Kconfig | 4 ++
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-gate.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/clk.h | 13 ++++++++
> 4 files changed, 96 insertions(+), 0 deletions(-)
> create mode 100644 drivers/clk/clk-gate.c
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index d8313d7..a78967c 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -12,3 +12,7 @@ config GENERIC_CLK
> config GENERIC_CLK_FIXED
> bool
> depends on GENERIC_CLK
> +
> +config GENERIC_CLK_GATE
> + bool
> + depends on GENERIC_CLK
I see zero documentation on what a "gated clock" is supposed to be or
how it works, and there are zero comments in the code. It's kind of
hard to review that way, and even harder to use.
g.
^ permalink raw reply
* [PATCH v2 6/7] clk: Add initial WM831x clock driver
From: Grant Likely @ 2011-09-25 4:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316730422-20027-7-git-send-email-mturquette@ti.com>
On Thu, Sep 22, 2011 at 03:27:01PM -0700, Mike Turquette wrote:
> From: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> The WM831x and WM832x series of PMICs contain a flexible clocking
> subsystem intended to provide always on and system core clocks. It
> features:
>
> - A 32.768kHz crystal oscillator which can optionally be used to pass
> through an externally generated clock.
> - A FLL which can be clocked from either the 32.768kHz oscillator or
> the CLKIN pin.
> - A CLKOUT pin which can bring out either the oscillator or the FLL
> output.
> - The 32.768kHz clock can also optionally be brought out on the GPIO
> pins of the device.
>
> This driver fully supports the 32.768kHz oscillator and CLKOUT. The FLL
> is supported only in AUTO mode, the full flexibility of the FLL cannot
> currently be used. The use of clock references other than the internal
> oscillator is not currently supported, and since clk_set_parent() is not
> implemented in the generic clock API the clock tree configuration cannot
> be changed at runtime.
>
> Due to a lack of access to systems where the core SoC has been converted
> to use the generic clock API this driver has been compile tested only.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Signed-off-by: Mike Turquette <mturquette@ti.com>
A few minor comments below. Otherwise looks fine to me.
> +static __devinit int wm831x_clk_probe(struct platform_device *pdev)
> +{
> + struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent);
> + struct wm831x_clk *clkdata;
> + int ret;
> +
> + clkdata = kzalloc(sizeof(*clkdata), GFP_KERNEL);
If devm_kzalloc() is used, then all the kfree unwinding can be
dropped.
> +static int __init wm831x_clk_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&wm831x_clk_driver);
> + if (ret != 0)
> + pr_err("Failed to register WM831x clock driver: %d\n", ret);
> +
> + return ret;
No need for this song-and-dance. The driver core is pretty well
debugged. Just use "return platform_driver_register(...);"
g.
^ permalink raw reply
* [PATCH v2 0/7] Add a generic struct clk
From: Grant Likely @ 2011-09-25 4:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316730422-20027-1-git-send-email-mturquette@ti.com>
On Thu, Sep 22, 2011 at 03:26:55PM -0700, Mike Turquette wrote:
> Hi all,
>
> The goal of this series is to provide a cross-platform clock framework
> that platforms can use to model their clock trees and perform common
> operations on them. Currently everyone re-invents their own clock tree
> inside platform code which makes it impossible for drivers to use the
> clock APIs safely across many platforms and for distro's to compile
> multi-platform kernels which all redefine struct clk and its operations.
>
> This is the second version of the common clock patches which were
> originally posted by Jeremy Kerr and then re-posted with some additional
> patches by Mark Brown. Mark's re-post didn't have any changes done to
> the original four patches from Jeremy which is why this series is "v2".
>
> The changes in this series are minimal: I've folded in some of Mark's
> fixes and most of the comments posted to his series as well as rebasing
> on top of v3.1-rc7. The design and functionality hasn't changed much
> since Jeremy posted v1 of this series. Propagating the rate change up
> to the parent has been removed from clk_set_rate since that needs some
> more thought. I also dropped Mark's change to append a device's name to
> a clk name since device tree might solve this neatly. Again more
> discussion around that would be good.
>
> v1 of the series can be found at,
> http://article.gmane.org/gmane.linux.kernel/1143182
>
> Mark's re-post (v1+) can be found at,
> http://article.gmane.org/gmane.linux.ports.arm.kernel/129889
Looks good at first review to me. I had a few comments, but nothing
major. It really needs some documentation though.
g.
^ permalink raw reply
* [PATCH v3 3/6] iommu/msm: announce supported page sizes
From: David Brown @ 2011-09-25 5:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316195506-9777-4-git-send-email-ohad@wizery.com>
On Fri, Sep 16, 2011 at 08:51:43PM +0300, Ohad Ben-Cohen wrote:
> Let the IOMMU core know we support 4KiB, 64KiB, 1MiB and 16MiB page sizes.
>
> This way the IOMMU core can split any arbitrary-sized physically
> contiguous regions (that it needs to map) as needed.
>
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> Cc: David Brown <davidb@codeaurora.org>
> Cc: Stepan Moskovchenko <stepanm@codeaurora.org>
> ---
> drivers/iommu/msm_iommu.c | 8 +++++++-
Acked-by: David Brown <davidb@codeaurora.org>
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* [PATCH v2 1/7] clk: Add a generic clock infrastructure
From: Turquette, Mike @ 2011-09-25 5:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110925035536.GN24631@ponder.secretlab.ca>
On Sat, Sep 24, 2011 at 8:55 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Sep 22, 2011 at 03:26:56PM -0700, Mike Turquette wrote:
>> From: Jeremy Kerr <jeremy.kerr@canonical.com>
>>
>> We currently have ~21 definitions of struct clk in the ARM architecture,
>> each defined on a per-platform basis. This makes it difficult to define
>> platform- (or architecture-) independent clock sources without making
>> assumptions about struct clk, and impossible to compile two
>> platforms with different struct clks into a single image.
>>
>> This change is an effort to unify struct clk where possible, by defining
>> a common struct clk, and a set of clock operations. Different clock
>> implementations can set their own operations, and have a standard
>> interface for generic code. The callback interface is exposed to the
>> kernel proper, while the clock implementations only need to be seen by
>> the platform internals.
>>
>> The interface is split into two halves:
>>
>> ?* struct clk, which is the generic-device-driver interface. This
>> ? ?provides a set of functions which drivers may use to request
>> ? ?enable/disable, query or manipulate in a hardware-independent manner.
>>
>> ?* struct clk_hw and struct clk_hw_ops, which is the hardware-specific
>> ? ?interface. Clock drivers implement the ops, which allow the core
>> ? ?clock code to implement the generic 'struct clk' API.
>>
>> This allows us to share clock code among platforms, and makes it
>> possible to dynamically create clock devices in platform-independent
>> code.
>>
>> Platforms can enable the generic struct clock through
>> CONFIG_GENERIC_CLK. In this case, the clock infrastructure consists of a
>> common, opaque struct clk, and a set of clock operations (defined per
>> type of clock):
>>
>> ? struct clk_hw_ops {
>> ? ? ? int ? ? ? ? ? ? (*prepare)(struct clk_hw *);
>> ? ? ? void ? ? ? ? ? ?(*unprepare)(struct clk_hw *);
>> ? ? ? int ? ? ? ? ? ? (*enable)(struct clk_hw *);
>> ? ? ? void ? ? ? ? ? ?(*disable)(struct clk_hw *);
>> ? ? ? unsigned long ? (*recalc_rate)(struct clk_hw *);
>> ? ? ? int ? ? ? ? ? ? (*set_rate)(struct clk_hw *,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long, unsigned long *);
>> ? ? ? long ? ? ? ? ? ?(*round_rate)(struct clk_hw *, unsigned long);
>> ? ? ? int ? ? ? ? ? ? (*set_parent)(struct clk_hw *, struct clk *);
>> ? ? ? struct clk * ? ?(*get_parent)(struct clk_hw *);
>> ? };
>>
>> Platform clock code can register a clock through clk_register, passing a
>> set of operations, and a pointer to hardware-specific data:
>>
>> ? struct clk_hw_foo {
>> ? ? ? struct clk_hw clk;
>> ? ? ? void __iomem *enable_reg;
>> ? };
>>
>> ? #define to_clk_foo(c) offsetof(c, clk_hw_foo, clk)
>>
>> ? static int clk_foo_enable(struct clk_hw *clk)
>> ? {
>> ? ? ? struct clk_foo *foo = to_clk_foo(clk);
>> ? ? ? raw_writeb(foo->enable_reg, 1);
>> ? ? ? return 0;
>> ? }
>>
>> ? struct clk_hw_ops clk_foo_ops = {
>> ? ? ? .enable = clk_foo_enable,
>> ? };
>>
>> And in the platform initialisation code:
>>
>> ? struct clk_foo my_clk_foo;
>>
>> ? void init_clocks(void)
>> ? {
>> ? ? ? my_clk_foo.enable_reg = ioremap(...);
>>
>> ? ? ? clk_register(&clk_foo_ops, &my_clk_foo, NULL);
>
> Shouldn't this be:
>
> ? ? ? ?clk_register(&clk_foo_ops, &my_clk_foo->clk, NULL);
>
> ?
>
> Also, this documentation would be good to have in the Documentation
> directory instead of lost in a commit header.
Thanks for your review Grant. Will fix the changelog and add proper
Documentation/ in the next round.
Regards,
Mike
> Otherwise looks okay to me.
>
> Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
>
>
^ permalink raw reply
* [PATCH v2 4/7] clk: Add simple gated clock
From: Turquette, Mike @ 2011-09-25 5:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110925040217.GO24631@ponder.secretlab.ca>
On Sat, Sep 24, 2011 at 9:02 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Sep 22, 2011 at 03:26:59PM -0700, Mike Turquette wrote:
>> From: Jeremy Kerr <jeremy.kerr@canonical.com>
>>
>> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
>> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
>> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
>> Signed-off-by: Mike Turquette <mturquette@ti.com>
>> ---
>> Changes since v1:
>> Add copyright header
>> Fold in Jamie's patch for set-to-disable clks
>> Use BIT macro instead of shift
>>
>> ?drivers/clk/Kconfig ? ?| ? ?4 ++
>> ?drivers/clk/Makefile ? | ? ?1 +
>> ?drivers/clk/clk-gate.c | ? 78 ++++++++++++++++++++++++++++++++++++++++++++++++
>> ?include/linux/clk.h ? ?| ? 13 ++++++++
>> ?4 files changed, 96 insertions(+), 0 deletions(-)
>> ?create mode 100644 drivers/clk/clk-gate.c
>>
>> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
>> index d8313d7..a78967c 100644
>> --- a/drivers/clk/Kconfig
>> +++ b/drivers/clk/Kconfig
>> @@ -12,3 +12,7 @@ config GENERIC_CLK
>> ?config GENERIC_CLK_FIXED
>> ? ? ? bool
>> ? ? ? depends on GENERIC_CLK
>> +
>> +config GENERIC_CLK_GATE
>> + ? ? bool
>> + ? ? depends on GENERIC_CLK
>
> I see zero documentation on what a "gated clock" is supposed to be or
> how it works, and there are zero comments in the code. ?It's kind of
> hard to review that way, and even harder to use.
Will add Documentation and re-post.
Thanks,
Mike
> g.
>
^ permalink raw reply
* [PATCH v2 6/7] clk: Add initial WM831x clock driver
From: Turquette, Mike @ 2011-09-25 5:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110925040836.GP24631@ponder.secretlab.ca>
On Sat, Sep 24, 2011 at 9:08 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Sep 22, 2011 at 03:27:01PM -0700, Mike Turquette wrote:
>> From: Mark Brown <broonie@opensource.wolfsonmicro.com>
>>
>> The WM831x and WM832x series of PMICs contain a flexible clocking
>> subsystem intended to provide always on and system core clocks. ?It
>> features:
>>
>> - A 32.768kHz crystal oscillator which can optionally be used to pass
>> ? through an externally generated clock.
>> - A FLL which can be clocked from either the 32.768kHz oscillator or
>> ? the CLKIN pin.
>> - A CLKOUT pin which can bring out either the oscillator or the FLL
>> ? output.
>> - The 32.768kHz clock can also optionally be brought out on the GPIO
>> ? pins of the device.
>>
>> This driver fully supports the 32.768kHz oscillator and CLKOUT. ?The FLL
>> is supported only in AUTO mode, the full flexibility of the FLL cannot
>> currently be used. ?The use of clock references other than the internal
>> oscillator is not currently supported, and since clk_set_parent() is not
>> implemented in the generic clock API the clock tree configuration cannot
>> be changed at runtime.
>>
>> Due to a lack of access to systems where the core SoC has been converted
>> to use the generic clock API this driver has been compile tested only.
>>
>> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
>> Signed-off-by: Mike Turquette <mturquette@ti.com>
>
> A few minor comments below. ?Otherwise looks fine to me.
>
>> +static __devinit int wm831x_clk_probe(struct platform_device *pdev)
>> +{
>> + ? ? struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent);
>> + ? ? struct wm831x_clk *clkdata;
>> + ? ? int ret;
>> +
>> + ? ? clkdata = kzalloc(sizeof(*clkdata), GFP_KERNEL);
>
> If devm_kzalloc() is used, then all the kfree unwinding can be
> dropped.
>
>> +static int __init wm831x_clk_init(void)
>> +{
>> + ? ? int ret;
>> +
>> + ? ? ret = platform_driver_register(&wm831x_clk_driver);
>> + ? ? if (ret != 0)
>> + ? ? ? ? ? ? pr_err("Failed to register WM831x clock driver: %d\n", ret);
>> +
>> + ? ? return ret;
>
> No need for this song-and-dance. ?The driver core is pretty well
> debugged. ?Just use "return platform_driver_register(...);"
Grant,
Thanks for the review.
Mark,
I know you're not carrying this whole set of patches but do you want
to rework this and resend or do you just want me to fix it up?
Changes are trivial if you don't want to touch it.
Regards,
Mike
> g.
>
^ permalink raw reply
* I-cache/D-cache inconsistency issue with page cache
From: Catalin Marinas @ 2011-09-25 9:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110924094734.GC17169@n2100.arm.linux.org.uk>
On 24 September 2011 10:47, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sat, Sep 24, 2011 at 11:35:44AM +0200, Mike Hommey wrote:
>> On Fri, Sep 23, 2011 at 08:39:41PM +0100, Russell King - ARM Linux wrote:
>> > On Fri, Sep 23, 2011 at 01:57:21PM +0200, Mike Hommey wrote:
>> > > We've been hitting random crashes at startup with Firefox on tegras
>> > > (under Android), and narrowed it down to a I-cache/D-cache
>> > > inconsistency. A reduced testcase of the issue looks like the following
>> > > (compile as ARM, not Thumb):
>> >
>> > If you write code at run time, you need to use the sys_cacheflush
>> > API to ensure that it's properly synchronized with the I-cache. ?It's
>> > a well known issue, and it applies to any harvard cache structured
>> > CPU which doesn't automatically ensure coherence (which essentially
>> > means all ARMs.)
>>
>> I do agree it's reasonable to have applications doing that to handle
>> cache synchronization themselves. I wrote such in my message. But I
>> think the kernel should make sure that its page cache is fresh when
>> it maps it PROT_EXEC. I think it's unreasonable to expect applications
>> doing mmap(PROT_WRITE), inflate, munmap, something, mmap(PROT_EXEC),
>> and execute something there to have to handle cache synchronisation
>> themselves. Especially when it's very CPU dependent (the testcase does
>> not even fail on all ARMs, only tegras, apparently). I'm not talking
>> actual code generation here, which needs platform-dependent behaviour.
>
> Ok. ?Which kernel are you trying this with, and which CPU (please
> confirm Cortex-A9)?
I had a discussion on Friday with the Firefox guys here in ARM. We
need to do some investigation next week but some random unverified
thoughts (that's on A9) - the scenario seems to be that a library
decompresses some data to a file using mmap(write) (which happens to
be code but it doesn't need to know that) while some other application
part tries, at a later time, to execute code in the same file using
mmap(exec).
By default, a new page cache page is dirty. At a first look,
mmap(write) and further access would not trigger a cache operation in
__sync_icache_dcache() and the page is still marked as dirty. Later
on, when the page is munmap'ed and mmap'ed(exec),
__sync_icache_dcache() (during fault processing) would flush the
D-cache and invalidate the I-cache, while marking the page 'clean'.
I wonder whether during the first mmap(write) and uncompressing, the
'clean' state could be set (maybe some flush_dcache_page) call. This
state would be preserved in the page cache page status and a
subsequent __sync_icache_dcache(), even from a different file, would
just notice that the page is 'clean'.
As I said, just some thoughts, I haven't tested this theory yet.
--
Catalin
^ permalink raw reply
* I-cache/D-cache inconsistency issue with page cache
From: Russell King - ARM Linux @ 2011-09-25 10:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHkRjk42PoRJrRb3vBdQCCno_LoMSdBrHSQ9xkiFcyB16PYkaw@mail.gmail.com>
On Sun, Sep 25, 2011 at 10:51:30AM +0100, Catalin Marinas wrote:
> I had a discussion on Friday with the Firefox guys here in ARM. We
> need to do some investigation next week but some random unverified
> thoughts (that's on A9) - the scenario seems to be that a library
> decompresses some data to a file using mmap(write) (which happens to
> be code but it doesn't need to know that) while some other application
> part tries, at a later time, to execute code in the same file using
> mmap(exec).
>
> By default, a new page cache page is dirty. At a first look,
> mmap(write) and further access would not trigger a cache operation in
> __sync_icache_dcache() and the page is still marked as dirty. Later
> on, when the page is munmap'ed and mmap'ed(exec),
> __sync_icache_dcache() (during fault processing) would flush the
> D-cache and invalidate the I-cache, while marking the page 'clean'.
>
> I wonder whether during the first mmap(write) and uncompressing, the
> 'clean' state could be set (maybe some flush_dcache_page) call. This
> state would be preserved in the page cache page status and a
> subsequent __sync_icache_dcache(), even from a different file, would
> just notice that the page is 'clean'.
>
> As I said, just some thoughts, I haven't tested this theory yet.
Not quite. Whenever we establish any page in the system which is
executable, we always flush the D cache and entire I cache.
As I've already pointed out though, the report is against old kernels
which doesn't have this code, so there's no point us speculating about
it until the issue has been confirmed against a kernel which we expect
_not_ to have the issue in the first place (rather than one which we
_do_ expect it to go wrong.)
^ permalink raw reply
* [RFC 0/5] OMAP groundwork for IOMMU-based DMA API
From: Ohad Ben-Cohen @ 2011-09-25 10:58 UTC (permalink / raw)
To: linux-arm-kernel
Another step towards generic IOMMU support on OMAP: eliminating
the public omap-specific find_iommu_device API.
Instead, we now bind iommu clients with their respective iommu device
using a private iommu member which is added to ARM's dev_archdata.
With this in hand, generic IOMMU API users will now work on OMAP without
utilizing any omap-specific API.
The changes are tested on OMAP3 (with omap3isp) and OMAP4 (with
remoteproc/rpmsg).
This is still RFC (2nd patch is probably the least elegant).
Ohad Ben-Cohen (5):
ARM: dev_archdata: add private iommu extension
ARM: OMAP: omap_device: add a method to set iommu private archdata
ARM: OMAP: iommu: declare a private iommu binding struct
ARM: OMAP3: bind omap3isp_device to its iommu device
iommu/omap: eliminate the public omap_find_iommu_device() method
arch/arm/include/asm/device.h | 3 +
arch/arm/mach-omap2/devices.c | 7 +++
arch/arm/plat-omap/include/plat/iommu.h | 31 ++++++++++++-
arch/arm/plat-omap/include/plat/iovmm.h | 12 +++---
arch/arm/plat-omap/include/plat/omap_device.h | 6 +++
arch/arm/plat-omap/omap_device.c | 39 +++++++++++++++-
drivers/iommu/omap-iommu.c | 58 +++++++++++--------------
drivers/iommu/omap-iovmm.c | 31 +++++++++-----
drivers/media/video/omap3isp/isp.c | 30 ++-----------
drivers/media/video/omap3isp/isp.h | 2 -
drivers/media/video/omap3isp/ispccdc.c | 18 ++++----
drivers/media/video/omap3isp/ispstat.c | 8 ++--
drivers/media/video/omap3isp/ispvideo.c | 4 +-
13 files changed, 152 insertions(+), 97 deletions(-)
--
1.7.4.1
^ permalink raw reply
* [RFC PATCH 0/3] Add accurate boot timing to a Linux system
From: Bjorn Helgaas @ 2011-09-25 12:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316818998-30711-1-git-send-email-sjg@chromium.org>
On Fri, Sep 23, 2011 at 5:03 PM, Simon Glass <sjg@chromium.org> wrote:
> This experimental patch set adds boot timing to a Linux system. The
> timing starts with the boot loader and extends through the kernel into
> user space to the completion of the boot process. The timing starts when
> the system leaves reset, not later when the kernel starts.
>
> The concept is:
> - Boot loader records a timestamp for key events during its operation
> - These timestamps are passed to Linux, which adds more as it boots
> - These timestamps are made available to user space, where more
> timestamps are added as init does its job
> - Finally the whole record is collected by a user-space script run at
> the end of init. This is fed back through some mechanism to monitor
> boot time in the field.
I think this is a cool idea. It's quite difficult to extract this
sort of information today, and making it easily and consistently
available should help focus attention and improve things.
There are difficult issues about which clock to use, how to correlate
bootloader & kernel timestamps, how to make sure the timestamps stay
sensible even when we use hwclock, ntp, etc., but I think it's worth
pushing on this to see how far you can go.
Bjorn
^ permalink raw reply
* [RFC PATCH 2/3] bootstage: Insert bootstage_mark to record timing for bootup.
From: Bjorn Helgaas @ 2011-09-25 12:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316818998-30711-3-git-send-email-sjg@chromium.org>
On Fri, Sep 23, 2011 at 5:03 PM, Simon Glass <sjg@chromium.org> wrote:
> From: Da Zheng <zhengda@chromium.org>
>
> From: Da Zheng <zhengda@chromium.org>
>
> This inserts calls to bootstage_mark() to timestamp the kernel moving
> between the different initcall levels.
>
> Signed-off-by: Da Zheng <zhengda@chromium.com>
> ---
> ?init/bootstage.c | ? 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> ?init/main.c ? ? ?| ? ?7 +++++++
> ?2 files changed, 56 insertions(+), 0 deletions(-)
>
> diff --git a/init/bootstage.c b/init/bootstage.c
> index 79e9761..6f4668f 100644
> --- a/init/bootstage.c
> +++ b/init/bootstage.c
> @@ -226,3 +226,52 @@ static int __init bootstage_init(void)
> ?}
>
> ?postcore_initcall(bootstage_init);
> +
> +static int __init post_core_initcall(void)
> +{
> + ? ? ? bootstage_mark("core_initcall");
> + ? ? ? return 0;
> +}
> +core_initcall_sync(post_core_initcall);
I'm sure you considered just instrumenting do_one_initcall() rather
than adding these individual initcall marks? That would get you a lot
more timestamps (maybe too many), and would help identify individual
drivers, which I suspect will be common offenders.
Bjorn
^ permalink raw reply
* I-cache/D-cache inconsistency issue with page cache
From: Catalin Marinas @ 2011-09-25 15:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110925103445.GA22455@n2100.arm.linux.org.uk>
On 25 September 2011 11:34, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Sep 25, 2011 at 10:51:30AM +0100, Catalin Marinas wrote:
>> I had a discussion on Friday with the Firefox guys here in ARM. We
>> need to do some investigation next week but some random unverified
>> thoughts (that's on A9) - the scenario seems to be that a library
>> decompresses some data to a file using mmap(write) (which happens to
>> be code but it doesn't need to know that) while some other application
>> part tries, at a later time, to execute code in the same file using
>> mmap(exec).
>>
>> By default, a new page cache page is dirty. At a first look,
>> mmap(write) and further access would not trigger a cache operation in
>> __sync_icache_dcache() and the page is still marked as dirty. Later
>> on, when the page is munmap'ed and mmap'ed(exec),
>> __sync_icache_dcache() (during fault processing) would flush the
>> D-cache and invalidate the I-cache, while marking the page 'clean'.
>>
>> I wonder whether during the first mmap(write) and uncompressing, the
>> 'clean' state could be set (maybe some flush_dcache_page) call. This
>> state would be preserved in the page cache page status and a
>> subsequent __sync_icache_dcache(), even from a different file, would
>> just notice that the page is 'clean'.
>>
>> As I said, just some thoughts, I haven't tested this theory yet.
>
> Not quite. ?Whenever we establish any page in the system which is
> executable, we always flush the D cache and entire I cache.
We flush the D-cache only if the page was not marked 'clean'. Is there
any chance that the page gets marked as clean before the first part of
the application wrote the data (uncompressing) via a mmap(write)
mapping? If this would happen, a subsequent mmap(exec) of the same
page (as the kernel would most likely find it in the page cache) would
find it 'clean' and avoid the D-cache flushing.
> As I've already pointed out though, the report is against old kernels
> which doesn't have this code, so there's no point us speculating about
> it until the issue has been confirmed against a kernel which we expect
> _not_ to have the issue in the first place (rather than one which we
> _do_ expect it to go wrong.)
Yes, they should definitely try a more recent kernel.
--
Catalin
^ permalink raw reply
* I-cache/D-cache inconsistency issue with page cache
From: Russell King - ARM Linux @ 2011-09-25 19:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHkRjk7u3RQxUSkU+DP0fD4qd2VzYB0eQA_NpVFebNtBcR6ysQ@mail.gmail.com>
On Sun, Sep 25, 2011 at 04:26:42PM +0100, Catalin Marinas wrote:
> On 25 September 2011 11:34, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > As I've already pointed out though, the report is against old kernels
> > which doesn't have this code, so there's no point us speculating about
> > it until the issue has been confirmed against a kernel which we expect
> > _not_ to have the issue in the first place (rather than one which we
> > _do_ expect it to go wrong.)
>
> Yes, they should definitely try a more recent kernel.
As the kernels which have been reported as having the issue do not
have __sync_icache_dcache, all bets are off for things behaving
correctly. I don't see any point speculating further until the
problem is confirmed on a kernel _with_ __sync_icache_dcache.
^ permalink raw reply
* [PATCH 3/4] iommu/exynos: Add iommu driver for Exynos4 Platforms
From: KyongHo Cho @ 2011-09-26 0:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110924093851.GB17169@n2100.arm.linux.org.uk>
Hi.
On Sat, Sep 24, 2011 at 6:38 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sat, Sep 24, 2011 at 07:35:45AM +0000, ??? wrote:
>> +# EXYNOS IOMMU support
>> +config EXYNOS_IOMMU
>> + bool "Exynos IOMMU Support"
>> + depends on ARCH_EXYNOS4
>> + select IOMMU_API
>> + select EXYNOS4_DEV_SYSMMU
>> + help
>
> Looks like your mailer converted tabs to one space. ?This makes it more
> difficult to read this patch as a whole. ?Don't expect good reviews as
> long as your mailer breaks stuff in this way.
Sorry for that :(
I will try another way to send patches correctly.
>> +static inline bool is_sysmmu_active(struct sysmmu_platdata *mmudata)
>> +{
>> + return atomic_read(&mmudata->activations) != 0;
>> +}
>
> This use of an atomic type is total rubbish. ?There's nothing magical
> about 'atomic_*' stuff which suddenly makes the rest of your code
> magically correct. ?Think about this:
>
> ? ? ? ?Thread0 ? ? ? ? ? ? ? ? Thread1
> ? ? ? ?atomic_inc_return(&v)
> ? ? ? ? ? ? ? ?== 1
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?atomic_dec_return(&v)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?== 0
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?disables MMU
> ? ? ? ?enables MMU
>
> Now you have the situation where the atomic value is '0' yet the MMU is
> enabled.
>
> Repeat after me: atomic types do not make code atomic. ?atomic types
> just make modifications to the type itself atomic. ?Never use atomic
> types for code synchronization.
>
You're right. I also found that atomic type is not required for that.
I think that the atomic type is need to be changed into a primitive type
And the drivers that handles System MMU need to care about the synchronization
of the state of System MMU.
Since the only way to handle System MMU from the outside of System MMU driver
is to use IOMMU API, the state of System MMU will be retained by IOMMU API
implementations for our System MMU.
>> +static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
>> +{
>> + /* SYSMMU is in blocked when interrupt occurred. */
>> + unsigned long addr;
>> + struct sysmmu_platdata *mmudata = dev_id;
>> + enum S5P_SYSMMU_INTERRUPT_TYPE itype;
>> +
>> + WARN_ON(!is_sysmmu_active(mmudata));
>> +
>> + itype = (enum S5P_SYSMMU_INTERRUPT_TYPE)
>> + __ffs(__raw_readl(mmudata->sfrbase + S5P_INT_STATUS));
>> +
>> + BUG_ON(!((itype >= 0) && (itype < 8)));
>> +
>> + dev_alert(mmudata->dev, "SYSTEM MMU FAULT!!!.\n");
>
> Do the exclaimation marks do anything for this error message?
>
It invokes report_iommu_fault() that is a handler of IOMMU fault.
Since faults are always handled by the fault handler,
the above message is not required and it is just a debug purpose.
As you mentioned, it does not mean anything and needed to be removed.
>> +
>> + if (!mmudata->domain)
>> + return IRQ_NONE;
>> +
>> + addr = __raw_readl(mmudata->sfrbase + fault_reg_offset[itype]);
>> +
>> + if (!report_iommu_fault(mmudata->domain, mmudata->owner, addr, itype)) {
>> + __raw_writel(1 << itype, mmudata->sfrbase + S5P_INT_CLEAR);
>> + dev_notice(mmudata->dev,
>> + "%s is resolved. Retrying translation.\n",
>
> So, this is a notice severity, yet we've printed at alert level. ?Either
> this condition satisfies being an alert or not.
It does not need to be notice severity and should be debug like above
alert message
because the fault handler has a chance to catch the fault that must not happen.
>
>> + sysmmu_fault_name[itype]);
>> + sysmmu_unblock(mmudata->sfrbase);
>> + } else {
>> + dev_notice(mmudata->dev, "%s is not handled.\n",
>> + sysmmu_fault_name[itype]);
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +void exynos_sysmmu_set_tablebase_pgd(struct device *owner, unsigned long pgd)
>> +{
>> + struct sysmmu_platdata *mmudata = NULL;
>> +
>> + while ((mmudata = get_sysmmu_data(owner, mmudata))) {
>> + if (is_sysmmu_active(mmudata)) {
>> + sysmmu_block(mmudata->sfrbase);
>> + __sysmmu_set_ptbase(mmudata->sfrbase, pgd);
>> + sysmmu_unblock(mmudata->sfrbase);
>> + dev_dbg(mmudata->dev, "New page table base is %p\n",
>> + (void *)pgd);
>
> If it's unsigned long use %08lx and don't cast.
>
Ok.
>> + if (request_irq(irq, exynos_sysmmu_irq, 0, dev_name(&pdev->dev),
>> + dev_get_platdata(dev))) {
>> + dev_err(dev, "Failed probing system MMU: "
>> + "failed to request irq.");
>> + ret = -ENOENT;
>> + goto err_irq;
>> + }
>> +
>> + get_platdata(dev)->clk = clk_get(dev, "sysmmu");
>
> It's rather horrible to see drivers modifying their platform data, rather
> than drivers using the driver data pointer in struct device. ?As far as
> drivers are concerned, the platform data should only ever be read.
>
Thank you. That's true.
All data that are set in the driver will be moved to drvdata.
>> +
>> + if (IS_ERR_OR_NULL(get_platdata(dev)->clk)) {
>
> IS_ERR() only please. ?The *only* allowable failure value for drivers to
> be concerned about here is if the return value is an error pointer.
> Drivers have no business rejecting a NULL pointer here.
>
Right. it will be changed.
>> +static int exynos_sysmmu_remove(struct platform_device *pdev)
>> +{
>> + return 0;
>> +}
>
> That doesn't look good. ?If you can't remove it don't provide the
> function at all.
>
Yes it does not required. I will remove it.
>> +
>> +int exynos_sysmmu_runtime_suspend(struct device *dev)
>> +{
>> + if (WARN_ON(is_sysmmu_active(dev_get_platdata(dev))))
>> + return -EFAULT;
>> +
>> + return 0;
>> +}
>> +
>> +int exynos_sysmmu_runtime_resume(struct device *dev)
>> +{
>> + if (WARN_ON(is_sysmmu_active(dev_get_platdata(dev))))
>> + return -EFAULT;
>> +
>> + return 0;
>> +}
>
> Is there much point to having runtime PM support in this driver?
>
A System MMU is included in the power domain of the device that is
master of System MMU.
If the system MMU dirver does not support power gating, the driver of
master device must care about System MMU's internal state.
- Master device driver must disable System MMU before suspending the
master device.
- Master device driver must check if the master device is resumed to
invalidate TLB
when updating the page table.
I thought that System MMU driver must care about everything about its
internal state
and outside of the drivers including the device driver of master
device do not need
to care about it.
>> +static int exynos_iommu_fault_handler(struct iommu_domain *domain,
>> + struct device *dev, unsigned long iova, int flags)
>> +{
>> + struct exynos_iommu_domain *priv = domain->priv;
>> +
>> + dev_err(priv->dev, "%s occured at %p(Page table base: %p)\n",
>> + sysmmu_fault_name[flags], (void *)iova,
>> + (void *)(__pa(priv->pgtable)));
>
> Again, you want to get rid of these casts and use a proper format string
> for what these are.
Ok.
>
>> +static int exynos_iommu_attach_device(struct iommu_domain *domain,
>> + ? ?struct device *dev)
>> +{
>> + struct exynos_iommu_domain *priv = domain->priv;
>> + int ret;
>> +
>> + spin_lock(&priv->lock);
>> +
>> + priv->dev = dev;
>> +
>> + ret = exynos_sysmmu_enable(domain);
>> + if (ret)
>> + return ret;
>> +
>> + spin_unlock(&priv->lock);
>> +
>> + return 0;
>
> This function has an indeterminant return state for the spinlock - it
> sometimes returns with it locked and sometimes with it unlocked. ?That's
> bad practice.
>
> Getting rid of the 'if (ret) return ret;' and changing this 'return 0;'
> to 'return ret;' is a simple way of solving it - and results in less
> complex code.
>
That is my mistake.
Thank you.
>> + while (entry != end_entry) {
>> + if (!write_lpage(entry, paddr)) {
>> + pr_err("%s: Failed to allocate large page"
>> + " entry.\n", __func__);
>
> Don't wrap error messages, irrespective of what checkpatch may tell you.
> They make them impossible to grep for.
You are right.
I missed that.
I will fix that.
Thank you for your detailed review.
Regards,
Cho KyongHo.
^ permalink raw reply
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
From: Abhijeet Dharmapurikar @ 2011-09-26 1:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E770B3E.9040401@arm.com>
On 09/19/2011 02:28 AM, Marc Zyngier wrote:
> On 19/09/11 00:20, Abhijeet Dharmapurikar wrote:
>> On 09/15/2011 09:52 AM, Marc Zyngier wrote:
...
>> > + * @devname: An ascii name for the claiming device
>> > + * @dev_id: A percpu cookie passed back to the handler function
>> > + *
>> > + * This call allocates interrupt resources, but doesn't
>> > + * automatically enable the interrupt. It has to be done on each
>> > + * CPU using enable_percpu_irq().
>> > + *
>> > + * Dev_id must be globally unique. It is a per-cpu variable, and
>> > + * the handler gets called with the interrupted CPU's instance of
>> > + * that variable.
>> > + */
>> > +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
>> > + const char *devname, void __percpu *dev_id)
>>
>> Can we add irqflags argument. I think it will be useful to pass flags,
>> at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
>> The chip could use a set_type callback for ppi's too.
>
> We're entering dangerous territory here. While this would work with the
> GIC (the interrupt type is at the distributor level), you could easily
> imagine an interrupt controller with the PPI configuration at the CPU
> interface level... In that case, calling set_type from __setup_irq()
> would end up doing the wrong thing, and I'd hate the API to give the
> idea it can do things it may not do in the end...
>
> Furthermore, do we actually have a GIC implementation where PPI
> configuration isn't read-only? I only know about the ARM implementation,
> and the Qualcomm may well be different (the spec says it's
> implementation defined).
Yes, you are exactly right, Qualcomm's GIC has configurable PPIs. The
default configuration for PPI's is level triggered, but we change the
timer PPI to edge trigger. Without this we wont even boot (no timer
interrupts). We do this trigger type setting in board specific code.
Although I agree with your concern, I would still request to provide a
facility to set the trigger flags. All the PPI's request will have that
argument set to zero, except for msm timer (and few other msm
interrupts). Additionally we can add that concern as a comment in
request_percpu_irq so the user of request_percpu_irq is aware of it.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
From: Abhijeet Dharmapurikar @ 2011-09-26 1:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E7FD5FD.5070103@codeaurora.org>
On 09/25/2011 06:31 PM, Abhijeet Dharmapurikar wrote:
> On 09/19/2011 02:28 AM, Marc Zyngier wrote:
>> On 19/09/11 00:20, Abhijeet Dharmapurikar wrote:
>>> On 09/15/2011 09:52 AM, Marc Zyngier wrote:
> ...
>>> > + * @devname: An ascii name for the claiming device
>>> > + * @dev_id: A percpu cookie passed back to the handler function
>>> > + *
>>> > + * This call allocates interrupt resources, but doesn't
>>> > + * automatically enable the interrupt. It has to be done on each
>>> > + * CPU using enable_percpu_irq().
>>> > + *
>>> > + * Dev_id must be globally unique. It is a per-cpu variable, and
>>> > + * the handler gets called with the interrupted CPU's instance of
>>> > + * that variable.
>>> > + */
>>> > +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
>>> > + const char *devname, void __percpu *dev_id)
>>>
>>> Can we add irqflags argument. I think it will be useful to pass flags,
>>> at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
>>> The chip could use a set_type callback for ppi's too.
>>
>> We're entering dangerous territory here. While this would work with the
>> GIC (the interrupt type is at the distributor level), you could easily
>> imagine an interrupt controller with the PPI configuration at the CPU
>> interface level... In that case, calling set_type from __setup_irq()
>> would end up doing the wrong thing, and I'd hate the API to give the
>> idea it can do things it may not do in the end...
>>
>> Furthermore, do we actually have a GIC implementation where PPI
>> configuration isn't read-only? I only know about the ARM implementation,
>> and the Qualcomm may well be different (the spec says it's
>> implementation defined).
>
> Yes, you are exactly right, Qualcomm's GIC has configurable PPIs. The
> default configuration for PPI's is level triggered, but we change the
> timer PPI to edge trigger. Without this we wont even boot (no timer
> interrupts). We do this trigger type setting in board specific code.
>
> Although I agree with your concern, I would still request to provide a
> facility to set the trigger flags. All the PPI's request will have that
> argument set to zero, except for msm timer (and few other msm
> interrupts). Additionally we can add that concern as a comment in
> request_percpu_irq so the user of request_percpu_irq is aware of it.
>
I need to correct myself a tad bit. As Russell King pointed in the other
email, the trigger type register in the GIC is banked per cpu for PPI
interrupts. So, on those lines, enable_percpu_irq should take this
irqflags parameter (and call set_type on the chip) instead of
request_percpu_irq.
Thanks,
Abhijeet
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* [PATCH 3/4] iommu/exynos: Add iommu driver for Exynos4 Platforms
From: Wanlong Gao @ 2011-09-26 2:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHQjnOPMRQetGF5VnwsMD=_JxWC5C-LQ5WRyZhPs2HtCzis2Tg@mail.gmail.com>
On 09/26/2011 08:44 AM, KyongHo Cho wrote:
> Hi.
>
> On Sat, Sep 24, 2011 at 6:38 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Sat, Sep 24, 2011 at 07:35:45AM +0000, ??? wrote:
>>> +# EXYNOS IOMMU support
>>> +config EXYNOS_IOMMU
>>> + bool "Exynos IOMMU Support"
>>> + depends on ARCH_EXYNOS4
>>> + select IOMMU_API
>>> + select EXYNOS4_DEV_SYSMMU
>>> + help
>>
>> Looks like your mailer converted tabs to one space. This makes it more
>> difficult to read this patch as a whole. Don't expect good reviews as
>> long as your mailer breaks stuff in this way.
> Sorry for that :(
> I will try another way to send patches correctly.
Use *git send-email* instead.
Thanks
^ permalink raw reply
* [PATCH v2 1/2] ARM: cache-l2x0: remove __init annotation from initialization functions
From: Barry Song @ 2011-09-26 2:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110923205500.GW17169@n2100.arm.linux.org.uk>
2011/9/24 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Mon, Sep 19, 2011 at 01:33:39PM +0800, Barry Song wrote:
>> Do you think the following is what you want?
>
> Almost. ?A couple of things:
>
> 1. Making the variables static means that folk like OMAP can't read the
> ? values at resume time from their assembly (forcing them to save and
> ? restore them, rather than using the already saved copy.)
>
> 2. It probably makes sense to make a structure out of the saved state
> ? information so that assembly code doesn't have to individually find
> ? the address of each variable. ?Instead, they can find the address of
> ? the structure (in physical memory if that's what they need) and use
> ? offsets.
>
> With (2) its probably worth adding a comment about the structure being
> used in platform code and it should only ever be appended to.
> (Alternatively, we could use the asm-offsets.h generation stuff to
> create preprocessor symbols for the offsets in the struct if we put the
> struct in a header file.)
>
Then the incremental changes:
diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h
b/arch/arm/include/asm/hardware/cache-l2x0.h
index 733606b..99f2793 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -96,6 +96,19 @@
#ifndef __ASSEMBLY__
extern void l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
extern int l2x0_of_init(__u32 aux_val, __u32 aux_mask);
+
+struct l2x0_regs {
+ unsigned long aux_ctrl;
+ /*
+ * Whether the following registers need to be saved/restored
+ * depends on platform
+ */
+ unsigned long tag_latency;
+ unsigned long data_latency;
+ unsigned long filter_start;
+ unsigned long filter_end;
+};
+
#endif
#endif
diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
index 16baba2..b8d72a8 100644
--- a/arch/arm/kernel/asm-offsets.c
+++ b/arch/arm/kernel/asm-offsets.c
@@ -20,6 +20,7 @@
#include <asm/thread_info.h>
#include <asm/memory.h>
#include <asm/procinfo.h>
+#include <asm/hardware/cache-l2x0.h>
#include <linux/kbuild.h>
/*
@@ -92,6 +93,14 @@ int main(void)
DEFINE(S_OLD_R0, offsetof(struct pt_regs, ARM_ORIG_r0));
DEFINE(S_FRAME_SIZE, sizeof(struct pt_regs));
BLANK();
+#ifdef CONFIG_CACHE_L2X0
+ DEFINE(L2X0_R_AUX_CTRL, offsetof(struct l2x0_regs, aux_ctrl));
+ DEFINE(L2X0_R_TAG_LATENCY, offsetof(struct l2x0_regs, tag_latency));
+ DEFINE(L2X0_R_DATA_LATENCY, offsetof(struct l2x0_regs, data_latency));
+ DEFINE(L2X0_R_FILTER_START, offsetof(struct l2x0_regs, filter_start));
+ DEFINE(L2X0_R_FILTER_END, offsetof(struct l2x0_regs, filter_end));
+ BLANK();
+#endif
#ifdef CONFIG_CPU_HAS_ASID
DEFINE(MM_CONTEXT_ID, offsetof(struct mm_struct, context.id));
BLANK();
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 98d03fb..b44f333 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -32,8 +32,8 @@ static void __iomem *l2x0_base;
static DEFINE_SPINLOCK(l2x0_lock);
static uint32_t l2x0_way_mask; /* Bitmask of active ways */
static uint32_t l2x0_size;
-static u32 l2x0_aux_ctrl;
-static u32 l2x0_tag_latency, l2x0_data_latency, l2x0_filter_start,
l2x0_filter_end;
+
+struct l2x0_regs l2x0_saved_regs;
struct l2x0_of_data {
void (*setup)(const struct device_node *, __u32 *, __u32 *);
@@ -364,7 +364,7 @@ void l2x0_init(void __iomem *base, __u32 aux_val,
__u32 aux_mask)
/* l2x0 controller is disabled */
writel_relaxed(aux, l2x0_base + L2X0_AUX_CTRL);
- l2x0_aux_ctrl = aux;
+ l2x0_saved_regs.aux_ctrl = aux;
l2x0_inv_all();
@@ -457,10 +457,10 @@ static void pl310_of_setup(const struct device_node *np,
static void __init pl310_save(void)
{
- l2x0_tag_latency = readl_relaxed(l2x0_base + L2X0_TAG_LATENCY_CTRL);
- l2x0_data_latency = readl_relaxed(l2x0_base + L2X0_DATA_LATENCY_CTRL);
- l2x0_filter_end = readl_relaxed(l2x0_base + L2X0_ADDR_FILTER_END);
- l2x0_filter_start = readl_relaxed(l2x0_base + L2X0_ADDR_FILTER_START);
+ l2x0_saved_regs.tag_latency = readl_relaxed(l2x0_base +
L2X0_TAG_LATENCY_CTRL);
+ l2x0_saved_regs.data_latency = readl_relaxed(l2x0_base +
L2X0_DATA_LATENCY_CTRL);
+ l2x0_saved_regs.filter_end = readl_relaxed(l2x0_base + L2X0_ADDR_FILTER_END);
+ l2x0_saved_regs.filter_start = readl_relaxed(l2x0_base +
L2X0_ADDR_FILTER_START);
}
static void l2x0_resume(void)
@@ -469,7 +469,7 @@ static void l2x0_resume(void)
/* restore aux ctrl and enable l2 */
l2x0_unlock(readl_relaxed(l2x0_base + L2X0_CACHE_ID));
- writel_relaxed(l2x0_aux_ctrl, l2x0_base + L2X0_AUX_CTRL);
+ writel_relaxed(l2x0_saved_regs.aux_ctrl, l2x0_base + L2X0_AUX_CTRL);
l2x0_inv_all();
@@ -481,10 +481,10 @@ static void pl310_resume(void)
{
if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & 1)) {
/* restore pl310 setup */
- writel_relaxed(l2x0_tag_latency, l2x0_base + L2X0_TAG_LATENCY_CTRL);
- writel_relaxed(l2x0_data_latency, l2x0_base + L2X0_DATA_LATENCY_CTRL);
- writel_relaxed(l2x0_filter_end, l2x0_base + L2X0_ADDR_FILTER_END);
- writel_relaxed(l2x0_filter_start, l2x0_base + L2X0_ADDR_FILTER_START);
+ writel_relaxed(l2x0_saved_regs.tag_latency, l2x0_base +
L2X0_TAG_LATENCY_CTRL);
+ writel_relaxed(l2x0_saved_regs.data_latency, l2x0_base +
L2X0_DATA_LATENCY_CTRL);
+ writel_relaxed(l2x0_saved_regs.filter_end, l2x0_base + L2X0_ADDR_FILTER_END);
+ writel_relaxed(l2x0_saved_regs.filter_start, l2x0_base +
L2X0_ADDR_FILTER_START);
}
l2x0_resume();
--
-barry
^ permalink raw reply related
* [PATCH v2] ARM: cache-l2x0: add resume entry for l2 in secure mode
From: Barry Song @ 2011-09-26 3:26 UTC (permalink / raw)
To: linux-arm-kernel
we save the l2x0 registers at the first initialization, and platform codes
can get them to restore l2x0 status after wakeup.
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
-v2:
define the structure l2x0_regs to hold all saved registers;
make saved copy of registers non-static so that platform codes can access them;
add asm_offset for l2x0 saved regs(l2x0_regs struct) so that asm codes can use
arch/arm/include/asm/hardware/cache-l2x0.h | 13 +++++
arch/arm/include/asm/outercache.h | 7 +++
arch/arm/kernel/asm-offsets.c | 9 +++
arch/arm/mm/cache-l2x0.c | 81 ++++++++++++++++++++++++---
4 files changed, 101 insertions(+), 9 deletions(-)
diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
index c48cb1e..4f9e81d 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -96,6 +96,19 @@
#ifndef __ASSEMBLY__
extern void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
extern int l2x0_of_init(__u32 aux_val, __u32 aux_mask);
+
+struct l2x0_regs {
+ unsigned long aux_ctrl;
+ /*
+ * Whether the following registers need to be saved/restored
+ * depends on platform
+ */
+ unsigned long tag_latency;
+ unsigned long data_latency;
+ unsigned long filter_start;
+ unsigned long filter_end;
+};
+
#endif
#endif
diff --git a/arch/arm/include/asm/outercache.h b/arch/arm/include/asm/outercache.h
index d838743..53426c6 100644
--- a/arch/arm/include/asm/outercache.h
+++ b/arch/arm/include/asm/outercache.h
@@ -34,6 +34,7 @@ struct outer_cache_fns {
void (*sync)(void);
#endif
void (*set_debug)(unsigned long);
+ void (*resume)(void);
};
#ifdef CONFIG_OUTER_CACHE
@@ -74,6 +75,12 @@ static inline void outer_disable(void)
outer_cache.disable();
}
+static inline void outer_resume(void)
+{
+ if (outer_cache.resume)
+ outer_cache.resume();
+}
+
#else
static inline void outer_inv_range(phys_addr_t start, phys_addr_t end)
diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
index 16baba2..b8d72a8 100644
--- a/arch/arm/kernel/asm-offsets.c
+++ b/arch/arm/kernel/asm-offsets.c
@@ -20,6 +20,7 @@
#include <asm/thread_info.h>
#include <asm/memory.h>
#include <asm/procinfo.h>
+#include <asm/hardware/cache-l2x0.h>
#include <linux/kbuild.h>
/*
@@ -92,6 +93,14 @@ int main(void)
DEFINE(S_OLD_R0, offsetof(struct pt_regs, ARM_ORIG_r0));
DEFINE(S_FRAME_SIZE, sizeof(struct pt_regs));
BLANK();
+#ifdef CONFIG_CACHE_L2X0
+ DEFINE(L2X0_R_AUX_CTRL, offsetof(struct l2x0_regs, aux_ctrl));
+ DEFINE(L2X0_R_TAG_LATENCY, offsetof(struct l2x0_regs, tag_latency));
+ DEFINE(L2X0_R_DATA_LATENCY, offsetof(struct l2x0_regs, data_latency));
+ DEFINE(L2X0_R_FILTER_START, offsetof(struct l2x0_regs, filter_start));
+ DEFINE(L2X0_R_FILTER_END, offsetof(struct l2x0_regs, filter_end));
+ BLANK();
+#endif
#ifdef CONFIG_CPU_HAS_ASID
DEFINE(MM_CONTEXT_ID, offsetof(struct mm_struct, context.id));
BLANK();
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 0d85d22..c22bee1 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -33,6 +33,14 @@ static DEFINE_SPINLOCK(l2x0_lock);
static uint32_t l2x0_way_mask; /* Bitmask of active ways */
static uint32_t l2x0_size;
+struct l2x0_regs l2x0_saved_regs;
+
+struct l2x0_of_data {
+ void (*setup)(const struct device_node *,__u32 *, __u32 *);
+ void (*save)(void);
+ void (*resume)(void);
+};
+
static inline void cache_wait_way(void __iomem *reg, unsigned long mask)
{
/* wait for cache operation by line or way to complete */
@@ -280,7 +288,7 @@ static void l2x0_disable(void)
spin_unlock_irqrestore(&l2x0_lock, flags);
}
-static void __init l2x0_unlock(__u32 cache_id)
+static void l2x0_unlock(__u32 cache_id)
{
int lockregs;
int i;
@@ -356,6 +364,8 @@ void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
/* l2x0 controller is disabled */
writel_relaxed(aux, l2x0_base + L2X0_AUX_CTRL);
+ l2x0_saved_regs.aux_ctrl = aux;
+
l2x0_inv_all();
/* enable L2X0 */
@@ -445,18 +455,64 @@ static void __init pl310_of_setup(const struct device_node *np,
}
}
+static void __init pl310_save(void)
+{
+ l2x0_saved_regs.tag_latency = readl_relaxed(l2x0_base + L2X0_TAG_LATENCY_CTRL);
+ l2x0_saved_regs.data_latency = readl_relaxed(l2x0_base + L2X0_DATA_LATENCY_CTRL);
+ l2x0_saved_regs.filter_end = readl_relaxed(l2x0_base + L2X0_ADDR_FILTER_END);
+ l2x0_saved_regs.filter_start = readl_relaxed(l2x0_base + L2X0_ADDR_FILTER_START);
+}
+
+static void l2x0_resume(void)
+{
+ if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & 1)) {
+ /* restore aux ctrl and enable l2 */
+ l2x0_unlock(readl_relaxed(l2x0_base + L2X0_CACHE_ID));
+
+ writel_relaxed(l2x0_saved_regs.aux_ctrl, l2x0_base + L2X0_AUX_CTRL);
+
+ l2x0_inv_all();
+
+ writel_relaxed(1, l2x0_base + L2X0_CTRL);
+ }
+}
+
+static void pl310_resume(void)
+{
+ if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & 1)) {
+ /* restore pl310 setup */
+ writel_relaxed(l2x0_saved_regs.tag_latency, l2x0_base + L2X0_TAG_LATENCY_CTRL);
+ writel_relaxed(l2x0_saved_regs.data_latency, l2x0_base + L2X0_DATA_LATENCY_CTRL);
+ writel_relaxed(l2x0_saved_regs.filter_end, l2x0_base + L2X0_ADDR_FILTER_END);
+ writel_relaxed(l2x0_saved_regs.filter_start, l2x0_base + L2X0_ADDR_FILTER_START);
+ }
+
+ l2x0_resume();
+}
+
+static const struct l2x0_of_data pl310_data = {
+ pl310_of_setup,
+ pl310_save,
+ pl310_resume,
+};
+
+static const struct l2x0_of_data l2x0_data = {
+ l2x0_of_setup,
+ NULL,
+ l2x0_resume,
+};
+
static const struct of_device_id l2x0_ids[] __initconst = {
- { .compatible = "arm,pl310-cache", .data = pl310_of_setup },
- { .compatible = "arm,l220-cache", .data = l2x0_of_setup },
- { .compatible = "arm,l210-cache", .data = l2x0_of_setup },
+ { .compatible = "arm,pl310-cache", .data = (void *)&pl310_data },
+ { .compatible = "arm,l220-cache", .data = (void *)&l2x0_data },
+ { .compatible = "arm,l210-cache", .data = (void *)&l2x0_data },
{}
};
int __init l2x0_of_init(__u32 aux_val, __u32 aux_mask)
{
struct device_node *np;
- void (*l2_setup)(const struct device_node *np,
- __u32 *aux_val, __u32 *aux_mask);
+ struct l2x0_of_data *data;
np = of_find_matching_node(NULL, l2x0_ids);
if (!np)
@@ -465,13 +521,20 @@ int __init l2x0_of_init(__u32 aux_val, __u32 aux_mask)
if (!l2x0_base)
return -ENOMEM;
+ data = of_match_node(l2x0_ids, np)->data;
+
/* L2 configuration can only be changed if the cache is disabled */
if (!(readl_relaxed(l2x0_base + L2X0_CTRL) & 1)) {
- l2_setup = of_match_node(l2x0_ids, np)->data;
- if (l2_setup)
- l2_setup(np, &aux_val, &aux_mask);
+ if (data->setup)
+ data->setup(np, &aux_val, &aux_mask);
}
+
+ if (data->save)
+ data->save();
+
l2x0_init(l2x0_base, aux_val, aux_mask);
+
+ outer_cache.resume = data->resume;
return 0;
}
#endif
--
1.7.1
Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog
^ permalink raw reply related
* [PATCH 1/2] ARM: pxa: fix building error by palm27x_lcd_init() when FB_PXA not defined
From: Eric Miao @ 2011-09-26 3:53 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Marek Vasut <marek.vasut@gmail.com>
Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
---
arch/arm/mach-pxa/include/mach/palm27x.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-pxa/include/mach/palm27x.h b/arch/arm/mach-pxa/include/mach/palm27x.h
index 0a5e5ea..f80bbe2 100644
--- a/arch/arm/mach-pxa/include/mach/palm27x.h
+++ b/arch/arm/mach-pxa/include/mach/palm27x.h
@@ -34,7 +34,7 @@ extern struct pxafb_mode_info palm_320x320_new_lcd_mode;
extern void __init palm27x_lcd_init(int power,
struct pxafb_mode_info *mode);
#else
-static inline void palm27x_lcd_init(int power, struct pxafb_mode_info *mode) {}
+#define palm27x_lcd_init(power, mode) do {} while (0)
#endif
#if defined(CONFIG_USB_GADGET_PXA27X) || \
--
1.7.4.1
^ permalink raw reply related
* [PATCH 2/2] ARM: pxa/z2: fix building error of pxa27x_cpu_suspend() no longer available
From: Eric Miao @ 2011-09-26 3:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1317009197-12578-1-git-send-email-eric.y.miao@gmail.com>
Cc: Vasily Khoruzhick <anarsoul@gmail.com>
Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
---
arch/arm/mach-pxa/z2.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
index 6c9275a..6fa4e74 100644
--- a/arch/arm/mach-pxa/z2.c
+++ b/arch/arm/mach-pxa/z2.c
@@ -679,6 +679,8 @@ static inline void z2_pmic_init(void) {}
#endif
#ifdef CONFIG_PM
+extern void pxa27x_cpu_pm_enter(suspend_state_t state);
+
static void z2_power_off(void)
{
/* We're using deep sleep as poweroff, so clear PSPR to ensure that
@@ -686,7 +688,8 @@ static void z2_power_off(void)
*/
PSPR = 0x0;
local_irq_disable();
- pxa27x_cpu_suspend(PWRMODE_DEEPSLEEP, PLAT_PHYS_OFFSET - PAGE_OFFSET);
+ pxa27x_set_pwrmode(PWRMODE_DEEPSLEEP);
+ pxa27x_cpu_pm_enter(PM_SUSPEND_MEM);
}
#else
#define z2_power_off NULL
--
1.7.4.1
^ permalink raw reply related
* [PATCH v2 1/3] ARM: imx: Add imx cpuidle driver
From: Rob Lee @ 2011-09-26 3:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110916213613.GA16381@n2100.arm.linux.org.uk>
Hello Russell,
On 16 September 2011 16:36, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Sep 16, 2011 at 12:27:48PM -0500, Robert Lee wrote:
>> Introduce a new cpuidle driver which provides the common cpuidle
>> functionality necessary for any imx soc cpuidle implementation.
>
> I think its probably about time we said no to this duplication of CPU
> idle infrastructure. ?There seems to be a common pattern appearing
> through all SoCs - they're all doing this:
>
>> +static int imx_enter_idle(struct cpuidle_device *dev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct cpuidle_state *state)
>> +{
>> + ? ? struct timeval before, after;
>> + ? ? int idle_time;
>> +
>> + ? ? local_irq_disable();
>> + ? ? local_fiq_disable();
>> +
>> + ? ? do_gettimeofday(&before);
>> +
>> + ? ? mach_cpuidle(dev, state);
>> +
>> + ? ? do_gettimeofday(&after);
>> +
>> + ? ? local_fiq_enable();
>> + ? ? local_irq_enable();
>> +
>> + ? ? idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
>> + ? ? ? ? ? ? (after.tv_usec - before.tv_usec);
>> +
>> + ? ? return idle_time;
>> +}
>
If I understand you correctly, it sounds like you are suggesting
adding cpuidle initialization functionality that is common for all ARM.
Did you mean just the above function or also the functionality found
in imx_cpuidle_init and imx_cpuidle_dev_init? For this common
ARM functionality, would a cpuidle.c file in arch/arm/common/ be the
right location?
> in some form, where 'do_gettimeofday' might be ktime_get() or
> getnstimeofday(). ?If we can standardize on which of the many time
> functions can be used (which would be a definite plus) we should move
> this out to common code.
Looking into the time keeping functionality more, of the time functions
you mentioned I think that ktime_get is preferable as its result it effectively
a monotonic time that won't be changed with calls to do_settimeofday
unlike the other two functions whose use of xtime only could result in
an error in the reported time difference on SMP systems.
>
> Maybe also the initialization code could be standardized and improved
> too - for instance, what if you boot with maxcpus=1 on a platform
> supporting 2 CPUs, and you bring CPU1 online from userspace? ?When
> these CPU idle initialization functions are called, only one CPU will
> be online, and as they use 'for_each_cpu(cpu_id, cpu_online_mask)'
> CPU1 will be missing the cpu idle init.
>
^ permalink raw reply
* Samsung GPIO changes break boot on Cragganmore
From: Kukjin Kim @ 2011-09-26 4:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110922135351.GC5173@opensource.wolfsonmicro.com>
Mark Brown wrote:
>
> Hi,
>
> Something in the recent GPIO changes have broken the boot for me on
> Cragganmore (S3C6410 based) systems. The boot dies before any console
> output is produced so it's hard to isolate exactly what went wrong but
> backing out:
>
> 5ec7414 ARM: SAMSUNG: Remove useless Samsung GPIO related CONFIGs
> 7c15788 ARM: SAMSUNG: Update the name of regarding Samsung GPIO
> bedcda6 gpio/samsung: gpio-samsung.c to support Samsung GPIOs
>
> fixes the system boot for me. The diff is rather substantial so it's
> hard to check by eyeball unfortunately.
>
Hi Mark,
Oh, ok. Let me test on SMDK6410 and will fix soon.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply
* [PATCH] S3C2443: Fix bit-reset in setrate of clk_armdiv
From: Kukjin Kim @ 2011-09-26 4:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201109222309.00068.heiko@sntech.de>
Heiko St?bner wrote:
>
> The changed statement should set the old armdiv bits to 0
> and not everything else, before setting the new value.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> arch/arm/mach-s3c2443/clock.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-s3c2443/clock.c b/arch/arm/mach-s3c2443/clock.c
> index 966bde5..27549c4 100644
> --- a/arch/arm/mach-s3c2443/clock.c
> +++ b/arch/arm/mach-s3c2443/clock.c
> @@ -124,7 +124,7 @@ static int s3c2443_armclk_setrate(struct clk *clk,
unsigned
> long rate)
> unsigned long clkcon0;
>
> clkcon0 = __raw_readl(S3C2443_CLKDIV0);
> - clkcon0 &= S3C2443_CLKDIV0_ARMDIV_MASK;
> + clkcon0 &= ~S3C2443_CLKDIV0_ARMDIV_MASK;
> clkcon0 |= val << S3C2443_CLKDIV0_ARMDIV_SHIFT;
> __raw_writel(clkcon0, S3C2443_CLKDIV0);
> }
> --
> 1.7.2.3
Hi Heiko,
Yes, you're right.
Applied, thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ 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