* [PATCH 2/3] arm64: Add Kconfig option for Samsung GH7 SoC family
From: Catalin Marinas @ 2014-02-12 18:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <B1132DDA-ECB1-4971-98AE-6DDDDD985CFB@codeaurora.org>
On 12 Feb 2014, at 16:25, Kumar Gala <galak@codeaurora.org> wrote:
> On Feb 12, 2014, at 4:38 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
>> On Tue, Feb 11, 2014 at 11:39:27PM +0000, Olof Johansson wrote:
>>> On Mon, Feb 10, 2014 at 10:29 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
>>>> This patch adds support for Samsung GH7 SoC in arm64/Kconfig.
>>>>
>>>> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>
>>> The overhead of building one more device tree isn't very large, and I
>>> don't see any other need to have a Kconfig entry per SoC at this time.
>>> It's of course up to Catalin, but you might just want to always
>>> compile all dts files instead.
>>
>> For arm64, I thought of getting rid of ARCH_* Kconfig entries entirely,
>> only that I haven't heard any strong opinion either way (in which case
>> I'll do it, with a risk of single Image getting bigger and bigger and
>> people needing smaller Image can trim their .config).
>
> One reason to keep around ARCH_* is for drivers shared between arm and arm64 that depend on it.
We already converted some of them (those depending on ARCH_VEXPRESS) to
just depend on ARM64. Ideally, at some point I?d like to see them as
defaulting to modules but I don?t think we are there yet (we had some
discussions at the last KS, I?m not sure anyone started looking into
this).
^ permalink raw reply
* [RFC/PATCH 0/3] Add devicetree scanning for randomness
From: Olof Johansson @ 2014-02-12 18:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140212174554.GM27395@titan.lakedaemon.net>
On Wed, Feb 12, 2014 at 9:45 AM, Jason Cooper <jason@lakedaemon.net> wrote:
> I brought this up at last weeks devicetree irc meeting. My goal is to
> provide early randomness for kaslr on ARM. Currently, my idea is modify
> the init script to save an additional random seed from /dev/urandom to
> /boot/random-seed.
>
> The bootloader would then load this file into ram, and pass the
> address/size to the kernel either via dt, or commandline. kaslr (run in
> the decompressor) would consume some of this randomness, and then
> random.c would consume the rest in a non-crediting initialization.
>
> While not ideal, it works in absence of an HRNG, and is no worse than
> the current situation of storing the seed in /var/lib/misc/random-seed.
> It also doesn't require modification of the bootloaders. Just an
> updated kernel, and update the bootloader environment to load the
> seed.
Hmm. There are some drawbacks with this -- it assumes you can "just
update the bootloader environment" which in general isn't easy to do.
Also, you can't assume that /boot is writable or exists on all
embedded systems.
In general, taking both runtime and system-dependend data and using
that to see entropy is a good idea. For example, device trees that
contain serial numbers and mac addresses for the individual system. I
think x86 feeds the DMI table in for similar purposes.
If that can be amended on some systems with a runtime seed (from
/boot), that's good but we can't rely on it.
-Olof
^ permalink raw reply
* [PATCH v2 0/9] ARM: multi-platform kconfig cleanup and mach-virt removal
From: Will Deacon @ 2014-02-12 18:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <44914279.FnJrZdyuux@wuerfel>
On Wed, Feb 12, 2014 at 06:07:56PM +0000, Arnd Bergmann wrote:
> On Wednesday 12 February 2014 17:16:21 Will Deacon wrote:
> > On Wed, Feb 12, 2014 at 05:11:45PM +0000, Marc Zyngier wrote:
> > > On 12/02/14 16:53, Arnd Bergmann wrote:
> > > >
> > > > On the topic of V6, we don't support CPU_V6T2 at all, though I assume that
> > > > there is an ARM1156 core tile for integrator and realview, and we support
> > > > running the V6/V6K parts with MMU turned off. Would all revisions of ARM1156
> > > > work with CPU_V6K and !MMU?
> > >
> > > CPU_V6 and !MMU should work. V6K might, but that requires close
> > > inspection (WFE, SEV shouldn't be used in UP context)...
> >
> > Well I can boot Debian on my 1136 r0p1 (no v6k) using a v6/v7 kernel, so the
> > lack of MMU shouldn't change that.
>
> I was more worried about ARM1156 not being a strict superset of V6K, i.e.
> stuff other than the MMU missing from it that the kernel would rely on.
ARM1156 doesn't implement v6k, so you mean subset, right? My point was that
1136 works fine, so the only remaining part should be the lack of MMU.
I can't think of anything else missing, but I've never actually tried
booting one!
Will
^ permalink raw reply
* [RFC/PATCH 0/3] Add devicetree scanning for randomness
From: Arnd Bergmann @ 2014-02-12 18:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140212174554.GM27395@titan.lakedaemon.net>
On Wednesday 12 February 2014 12:45:54 Jason Cooper wrote:
> I brought this up at last weeks devicetree irc meeting. My goal is to
> provide early randomness for kaslr on ARM. Currently, my idea is modify
> the init script to save an additional random seed from /dev/urandom to
> /boot/random-seed.
>
> The bootloader would then load this file into ram, and pass the
> address/size to the kernel either via dt, or commandline. kaslr (run in
> the decompressor) would consume some of this randomness, and then
> random.c would consume the rest in a non-crediting initialization.
I like the idea, but wouldn't it be easier to pass actual random data
using DT, rather than the address/size? That way we could even
use the same DT binding for passing randomness from the bootloader,
whereever it may have found that.
If the bootloader has internet connectivity, it could even mix in
some data from http://www.random.org/cgi-bin/randbyte?nbytes=256&format=f
;-)
Arnd
^ permalink raw reply
* [PATCH 2/3] PCI: ARM: add support for virtual PCI host controller
From: Jason Gunthorpe @ 2014-02-12 18:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140212181015.GJ29132@mudshark.cambridge.arm.com>
On Wed, Feb 12, 2014 at 06:10:15PM +0000, Will Deacon wrote:
> > AFAIK, the job is fairly simple, when you call pci_add_resource_offset
> > for memory compute the offset from
> > of_pci_range.pci_addr - of_pci_range.cpu_addr
> >
> > (or is it the other way around ?)
>
> I think it's the other way round: bus = cpu - offset, then Arnd's example of
> PCI bus 0 works out as: 0 = cpu - pci->mem_start.
That looks right to me
> I added that to my driver, but I get some weird looking bus addresses in
> dmesg:
>
> [ 0.307585] pci-arm-generic 40000000.pci: PCI host bridge to bus 0000:00
> [ 0.307601] pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
> [ 0.307615] pci_bus 0000:00: root bus resource [mem 0x00000000-0x3effffff] (bus address [0xffffffffbf000000-0xfffffffffdffffff])
>
> Looking at drivers/pci/probe.c, it seems to think that res->start - offset
> gives a bus address, which implies that the resources are indeed *CPU*
> addresses.
>
> Are you sure pci_add_resource_offset wants bus addresses?
Sorry, I wasn't clear: It accepts a cpu address in the struct
resource and an offset to convert back to a bus address.
You should compute 0 as the offset in the normal case, ie
of_pci_range.pci_addr and of_pci_range.cpu_addr should be identical,
which depends on the DT ranges being correct..
Jason
^ permalink raw reply
* [RFC/PATCH 0/3] Add devicetree scanning for randomness
From: Jason Gunthorpe @ 2014-02-12 18:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140212174554.GM27395@titan.lakedaemon.net>
On Wed, Feb 12, 2014 at 12:45:54PM -0500, Jason Cooper wrote:
> The bootloader would then load this file into ram, and pass the
> address/size to the kernel either via dt, or commandline. kaslr (run in
> the decompressor) would consume some of this randomness, and then
> random.c would consume the rest in a non-crediting initialization.
Sure is a neat idea, but I think in general it would probably be smart
to include the entire FDT blob in the early random pool, that way you
get MACs and other machine unique data too.
>From there it is a small step to encourage bootloaders to include
boot-time-variable data in the DT like like 'boot time of day', 'cycle
counter', 'random blob', etc.
Then you just need the bootloader to dump the random-seed file into a
DT property.
Or have the bootloader fetch randomness from any HWRNG it has a driver
for. (eg a TPM)
Jason
^ permalink raw reply
* [PATCH v2 0/9] ARM: multi-platform kconfig cleanup and mach-virt removal
From: Arnd Bergmann @ 2014-02-12 18:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140212181527.GK29132@mudshark.cambridge.arm.com>
On Wednesday 12 February 2014 18:15:27 Will Deacon wrote:
> On Wed, Feb 12, 2014 at 06:07:56PM +0000, Arnd Bergmann wrote:
> > On Wednesday 12 February 2014 17:16:21 Will Deacon wrote:
> > > On Wed, Feb 12, 2014 at 05:11:45PM +0000, Marc Zyngier wrote:
> > > > On 12/02/14 16:53, Arnd Bergmann wrote:
> > > > >
> > > > > On the topic of V6, we don't support CPU_V6T2 at all, though I assume that
> > > > > there is an ARM1156 core tile for integrator and realview, and we support
> > > > > running the V6/V6K parts with MMU turned off. Would all revisions of ARM1156
> > > > > work with CPU_V6K and !MMU?
> > > >
> > > > CPU_V6 and !MMU should work. V6K might, but that requires close
> > > > inspection (WFE, SEV shouldn't be used in UP context)...
> > >
> > > Well I can boot Debian on my 1136 r0p1 (no v6k) using a v6/v7 kernel, so the
> > > lack of MMU shouldn't change that.
> >
> > I was more worried about ARM1156 not being a strict superset of V6K, i.e.
> > stuff other than the MMU missing from it that the kernel would rely on.
>
> ARM1156 doesn't implement v6k, so you mean subset, right? My point was that
> 1136 works fine, so the only remaining part should be the lack of MMU.
I meant superset of the no-mmu subset of v6k. ARM1156 introduced Thumb2
support, so it's clearly not just a subset but also contains stuff that
wasn't part of v6k.
Arnd
^ permalink raw reply
* [PATCH 2/3] PCI: ARM: add support for virtual PCI host controller
From: Will Deacon @ 2014-02-12 18:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140212181913.GA16428@obsidianresearch.com>
On Wed, Feb 12, 2014 at 06:19:13PM +0000, Jason Gunthorpe wrote:
> On Wed, Feb 12, 2014 at 06:10:15PM +0000, Will Deacon wrote:
>
> > > AFAIK, the job is fairly simple, when you call pci_add_resource_offset
> > > for memory compute the offset from
> > > of_pci_range.pci_addr - of_pci_range.cpu_addr
> > >
> > > (or is it the other way around ?)
> >
> > I think it's the other way round: bus = cpu - offset, then Arnd's example of
> > PCI bus 0 works out as: 0 = cpu - pci->mem_start.
>
> That looks right to me
>
> > I added that to my driver, but I get some weird looking bus addresses in
> > dmesg:
> >
> > [ 0.307585] pci-arm-generic 40000000.pci: PCI host bridge to bus 0000:00
> > [ 0.307601] pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
> > [ 0.307615] pci_bus 0000:00: root bus resource [mem 0x00000000-0x3effffff] (bus address [0xffffffffbf000000-0xfffffffffdffffff])
> >
> > Looking at drivers/pci/probe.c, it seems to think that res->start - offset
> > gives a bus address, which implies that the resources are indeed *CPU*
> > addresses.
> >
> > Are you sure pci_add_resource_offset wants bus addresses?
>
> Sorry, I wasn't clear: It accepts a cpu address in the struct
> resource and an offset to convert back to a bus address.
>
> You should compute 0 as the offset in the normal case, ie
> of_pci_range.pci_addr and of_pci_range.cpu_addr should be identical,
> which depends on the DT ranges being correct..
Aha! That explains all of the confusion. I'll remove my homebrew
resource translation code then :)
Thanks,
Will
^ permalink raw reply
* [PATCH] arm64: smp: Add a memory barrier before we start secondary cores
From: Catalin Marinas @ 2014-02-12 18:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140212145644.GP28112@sirena.org.uk>
On 12 Feb 2014, at 14:56, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Feb 12, 2014 at 01:43:32PM +0000, Catalin Marinas wrote:
>
>> I think we can drop this until Vincent clarifies the synchronisation
>> requirements (still wondering whether spinlocks are needed).
>
> Even if they're not actually required for the topology code the fact
> that we're having to think about what is needed suggests that the code
> is going to end up being clearer and hence more maintainable going
> forwards if we add it.
I think we should aim to understand the code better rather than just
adding the spinlocks. Some simple questions:
- Who uses the topology information and when? (the scheduler? first time after or
before secondaries are started?)
- Who updates the topology information? (primary and secondary startup
code?)
- What about hotplug?
- Can any of the above happen in parallel on different CPUs?
Catalin
^ permalink raw reply
* [PATCH v3 2/7] clk: samsung: add infrastructure to register cpu clocks
From: Tomasz Figa @ 2014-02-12 18:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391788548-13056-3-git-send-email-thomas.ab@samsung.com>
Hi Thomas,
On 07.02.2014 16:55, Thomas Abraham wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
>
> The CPU clock provider supplies the clock to the CPU clock domain. The
> composition and organization of the CPU clock provider could vary among
> Exynos SoCs. A CPU clock provider can be composed of clock mux, dividers
> and gates. This patch defines a new clock type for CPU clock provider and
> adds infrastructure to register the CPU clock providers for Samsung
> platforms.
>
> In addition to this, the arm cpu clock provider for Exynos4210 and
> compatible SoCs is instantiated using the new cpu clock type. The clock
> configuration data for this clock is obtained from device tree. This
> implementation is reusable for Exynos4x12 and Exynos5250 SoCs as well.
>
> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> ---
> drivers/clk/samsung/Makefile | 2 +-
> drivers/clk/samsung/clk-cpu.c | 409 +++++++++++++++++++++++++++++++++++++++++
> drivers/clk/samsung/clk.h | 5 +
> 3 files changed, 415 insertions(+), 1 deletion(-)
> create mode 100644 drivers/clk/samsung/clk-cpu.c
[snip]
> diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c
> new file mode 100644
> index 0000000..673f620
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-cpu.c
> @@ -0,0 +1,409 @@
[snip]
> +#define SRC_CPU 0x0
> +#define STAT_CPU 0x200
> +#define DIV_CPU0 0x300
> +#define DIV_CPU1 0x304
> +#define DIV_STAT_CPU0 0x400
> +#define DIV_STAT_CPU1 0x404
> +
> +#define MAX_DIV 8
> +
> +#define EXYNOS4210_ARM_DIV1(base) ((readl(base + DIV_CPU0) & 0xf) + 1)
> +#define EXYNOS4210_ARM_DIV2(base) (((readl(base + DIV_CPU0) >> 28) & 0xf) + 1)
Those are 3-bit dividers, so the mask should be rather 0x7, shouldn't it?
Btw. Somehow I feel like this kind of macros is simply obfuscating code
without any real benefits. Could you rewrite them to simply take the
value of DIV_CPU0 register, without hiding readl behind the scenes?
> +
> +#define EXYNOS4210_DIV_CPU0(d5, d4, d3, d2, d1, d0) \
> + ((d5 << 24) | (d4 << 20) | (d3 << 16) | (d2 << 12) | \
> + (d1 << 8) | (d0 << 4))
> +#define EXYNOS4210_DIV_CPU1(d2, d1, d0) \
> + ((d2 << 8) | (d1 << 4) | (d0 << 0))
> +
> +#define EXYNOS4210_DIV1_HPM_MASK ((0x7 << 0) | (0x7 << 4))
> +#define EXYNOS4210_MUX_HPM_MASK (1 << 20)
[snip]
> +/* common round rate callback useable for all types of cpu clocks */
> +static long samsung_cpuclk_round_rate(struct clk_hw *hw,
> + unsigned long drate, unsigned long *prate)
> +{
> + struct clk *parent = __clk_get_parent(hw->clk);
> + unsigned long max_prate = __clk_round_rate(parent, UINT_MAX);
> + unsigned long t_prate, best_div = 1;
> + unsigned long delta, min_delta = UINT_MAX;
> +
> + do {
> + t_prate = __clk_round_rate(parent, drate * best_div);
> + delta = drate - (t_prate / best_div);
> + if (delta < min_delta) {
> + *prate = t_prate;
> + min_delta = delta;
> + }
> + if (!delta)
> + break;
> + best_div++;
> + } while ((drate * best_div) < max_prate && best_div <= MAX_DIV);
> +
> + return t_prate / best_div;
> +}
I think there is something wrong with the code above. You increment
best_div in every iteration of the loop and use it to calculate the
final best frequency. Shouldn't best_div be the divisor which was found
to give the least delta and so the loop should rather iterate on a
different variable and store best_div only if (delta < min_delta)
condition is true?
Anyway, I wonder if you couldn't somehow reuse the code from
drivers/clk/clk-divider.c...
> +
> +static unsigned long _calc_div(unsigned long prate, unsigned long drate)
> +{
> + unsigned long div = prate / drate;
> +
> + WARN_ON(div >= MAX_DIV);
> + return (!(prate % drate)) ? div-- : div;
> +}
> +
> +/* helper function to register a cpu clock */
> +static int __init samsung_cpuclk_register(unsigned int lookup_id,
Isn't the name a bit too generic? I'd say it should be
exynos_cpuclk_register(), since the implementation is rather Exynos
specific anyway.
> + const char *name, const char **parents,
> + unsigned int num_parents, void __iomem *base,
> + const struct samsung_cpuclk_soc_data *soc_data,
> + struct device_node *np, const struct clk_ops *ops)
> +{
> + struct samsung_cpuclk *cpuclk;
> + struct clk_init_data init;
> + struct clk *clk;
> + int ret;
> +
> + cpuclk = kzalloc(sizeof(*cpuclk), GFP_KERNEL);
> + if (!cpuclk) {
> + pr_err("%s: could not allocate memory for %s clock\n",
> + __func__, name);
> + return -ENOMEM;
> + }
> +
> + init.name = name;
> + init.flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_PARENT;
Why CLK_GET_RATE_NOCACHE? There is only one entity that can change rate
of this clock and it is this clock driver, so it can update the cache on
any change.
> + init.parent_names = parents;
> + init.num_parents = 1;
I believe this clock should take two clocks as its parents, because it
can dynamically switch between mout_apll and mout_mpll using mout_core mux.
> + init.ops = ops;
> +
> + cpuclk->hw.init = &init;
> + cpuclk->ctrl_base = base;
> +
> + if (soc_data && soc_data->parser) {
Is it even possible to instantiate this clock without soc_data and
parser function? Shouldn't it simply bail out instead?
> + ret = soc_data->parser(np, &cpuclk->data);
> + if (ret) {
> + pr_err("%s: error %d in parsing %s clock data",
> + __func__, ret, name);
> + ret = -EINVAL;
> + goto free_cpuclk;
> + }
> + cpuclk->offset = soc_data->offset;
> + init.ops = soc_data->ops;
> + }
> +
> + if (soc_data && soc_data->clk_cb) {
Same here. Does it make any sense to instantiate this clock without a
notifier callback?
> + cpuclk->clk_nb.notifier_call = soc_data->clk_cb;
> + if (clk_notifier_register(__clk_lookup(parents[0]),
> + &cpuclk->clk_nb)) {
> + pr_err("%s: failed to register clock notifier for %s\n",
> + __func__, name);
> + goto free_cpuclk_data;
> + }
> + }
> +
> + if (num_parents == 2) {
When num_parents could be other than 2?
> + cpuclk->alt_parent = __clk_lookup(parents[1]);
> + if (!cpuclk->alt_parent) {
> + pr_err("%s: could not lookup alternate parent %s\n",
> + __func__, parents[1]);
> + ret = -EINVAL;
> + goto free_cpuclk_data;
> + }
> + }
> +
> + clk = clk_register(NULL, &cpuclk->hw);
> + if (IS_ERR(clk)) {
> + pr_err("%s: could not register cpuclk %s\n", __func__, name);
> + ret = PTR_ERR(clk);
> + goto free_cpuclk_data;
> + }
> +
> + samsung_clk_add_lookup(clk, lookup_id);
> + return 0;
> +
> +free_cpuclk_data:
> + kfree(cpuclk->data);
> +free_cpuclk:
> + kfree(cpuclk);
> + return ret;
> +}
> +
> +static inline void _exynos4210_set_armclk_div(void __iomem *base,
> + unsigned long div)
I'd say it would be better to leave the decision about inlining to the
compiler.
> +{
> + writel((readl(base + DIV_CPU0) & ~0xf) | div, base + DIV_CPU0);
CORE_RATIO bitfield is 3-bit wide.
> + while (readl(base + DIV_STAT_CPU0) != 0)
> + ;
Wouldn't it be better to add some timeout and print an error to make
sure that even if something wrong happens the user will know that it
happened?
> +}
> +
> +static unsigned long exynos4210_armclk_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct samsung_cpuclk *armclk = to_samsung_cpuclk_hw(hw);
> + void __iomem *base = armclk->ctrl_base + armclk->offset;
> +
> + return parent_rate / EXYNOS4210_ARM_DIV1(base) /
> + EXYNOS4210_ARM_DIV2(base);
The code would be more readable if you read the register to a temporary
variable using readl() directly and then accessing its contents.
> +}
> +
> +/*
> + * This clock notifier is called when the frequency of the parent clock
> + * of armclk is to be changed. This notifier handles the setting up all
> + * the divider clocks, remux to temporary parent and handling the safe
> + * frequency levels when using temporary parent.
> + */
> +static int exynos4210_armclk_notifier_cb(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct clk_notifier_data *ndata = data;
> + struct samsung_cpuclk *armclk = to_samsung_cpuclk_nb(nb);
> + struct exynos4210_armclk_data *armclk_data;
> + unsigned long alt_prate, alt_div, div0, div1, mux_reg;
> + void __iomem *base;
> + bool need_safe_freq;
> +
> + armclk_data = armclk->data;
> + base = armclk->ctrl_base + armclk->offset;
> + alt_prate = clk_get_rate(armclk->alt_parent);
> +
> + if (event == POST_RATE_CHANGE)
> + goto e4210_armclk_post_rate_change;
This would look better if you separated the main configuration code from
the notifier callback, like this:
int ret = 0;
switch (event) {
case POST_RATE_CHANGE:
ret = exynos4210_armclk_post_rate_change(...);
break;
case PRE_RATE_CHANGE:
ret = exynos4210_armclk_pre_rate_change(...);
break;
}
return notifier_from_errno(ret);
By the way, I wonder if some of this couldn't simply happen in
.set_rate() op of this clock, since
> +
> + /* pre-rate change. find out the divider values to use for clock data */
> + while (armclk_data->prate != ndata->new_rate) {
> + if (armclk_data->prate == 0)
> + return NOTIFY_BAD;
> + armclk_data++;
> + }
> +
> + div0 = armclk_data->div0;
> + div1 = armclk_data->div1;
You should always use read-modify-write for such registers to preserve
reserved bits. This will also clean a bit safe frequency activation, see
below.
> + if (readl(base + SRC_CPU) & EXYNOS4210_MUX_HPM_MASK) {
> + div1 = readl(base + DIV_CPU1) & EXYNOS4210_DIV1_HPM_MASK;
> + div1 |= ((armclk_data->div1) & ~EXYNOS4210_DIV1_HPM_MASK);
> + }
> +
> + /*
> + * if the new and old parent clock speed is less than the clock speed
> + * of the alternate parent, then it should be ensured that at no point
> + * the armclk speed is more than the old_prate until the dividers are
> + * set.
> + */
> + need_safe_freq = ndata->old_rate < alt_prate &&
> + ndata->new_rate < alt_prate;
Are you sure this condition is correct? The parent clock (PLL) alone
doesn't fully determine the rate of ARM clock, because you are also
changing div_core. So you can end up with PLL going down, while ARM
clock going up, because the divisors are significantly lowered.
I think you should compare ARM clock rates here OR just remove div_core
configuration, keeping it as 0 (divide by 1) all the time, except when
safe frequency is active.
> + if (need_safe_freq) {
> + alt_div = _calc_div(alt_prate, ndata->old_rate);
> + _exynos4210_set_armclk_div(base, alt_div);
> + div0 |= alt_div;
> + }
You could move div0 and div1 calculation (including reading original
values of registers) here to remove "div0 |= alt_div;" line from the if
above and fully isolate ARM divisor setup code from changing other divisors.
> +
> + mux_reg = readl(base + SRC_CPU);
> + writel(mux_reg | (1 << 16), base + SRC_CPU);
Don't you need to hold the samsung clock spinlock when writing to this
register? It seems to control more muxes than just this one.
> + while (((readl(base + STAT_CPU) >> 16) & 0x7) != 2)
> + ;
> +
> + writel(div0, base + DIV_CPU0);
> + while (readl(base + DIV_STAT_CPU0) != 0)
> + ;
> + writel(div1, base + DIV_CPU1);
> + while (readl(base + DIV_STAT_CPU1) != 0)
> + ;
You seem to always perform the configuration in pre rate change
notifier, but original cpufreq code used to do it depending on whether
the new frequency is less than old or not, e.g.
static void exynos5250_set_frequency(unsigned int old_index,
unsigned int new_index)
{
if (old_index > new_index) {
set_clkdiv(new_index);
set_apll(new_index);
} else if (old_index < new_index) {
set_apll(new_index);
set_clkdiv(new_index);
}
}
set_clkdiv does the following:
1) set DIV_CPU0
2) wait for DIV_STAT_CPU0
3) set DIV_CPU1
4) wait for DIV_STAT_CPU1
and set_apll:
1) set parent to MPLL
2) wait for MUX_STAT_CPU
3) set APLL rate
4) set parent to APLL
5) wait for MUX_STAT_CPU
Note that higher index means lower frequency.
Btw. It would be nice to handle timeouts here as well, to prevent system
lockups.
> + return NOTIFY_OK;
> +
> +e4210_armclk_post_rate_change:
> + /* post-rate change event, re-mux back to primary parent */
> + mux_reg = readl(base + SRC_CPU);
> + writel(mux_reg & ~(1 << 16), base + SRC_CPU);
> + while (((readl(base + STAT_CPU) >> 16) & 0x7) != 1)
> + ;
nit: Too many tabs.
Timeout would be nice too.
> +
> + return NOTIFY_OK;
> +}
> +
> +static int exynos4210_armclk_set_rate(struct clk_hw *hw, unsigned long drate,
> + unsigned long prate)
> +{
> + struct samsung_cpuclk *armclk = to_samsung_cpuclk_hw(hw);
> + void __iomem *base = armclk->ctrl_base + armclk->offset;
> + unsigned long div;
> +
> + div = drate < prate ? _calc_div(prate, drate) : 0;
> + _exynos4210_set_armclk_div(base, div);
> + return 0;
Hmm, here you are supposed to set exactly the rate given to you in drate
argument. Core clock code calls your .round_rate() first and the rate
returned by it is what .set_rate() gets as drate. You can safely assume
that
> +}
> +
> +static const struct clk_ops exynos4210_armclk_clk_ops = {
> + .recalc_rate = exynos4210_armclk_recalc_rate,
> + .round_rate = samsung_cpuclk_round_rate,
> + .set_rate = exynos4210_armclk_set_rate,
> +};
> +
> +/*
> + * parse divider configuration data from dt for all the cpu clock domain
> + * clocks in exynos4210 and compatible SoC's.
> + */
> +static int __init exynos4210_armclk_parser(struct device_node *np, void **data)
> +{
> + struct exynos4210_armclk_data *tdata;
> + unsigned long cfg[10], row, col;
> + const struct property *prop;
> + const __be32 *val;
> + u32 cells;
> + int ret;
> +
> + if (of_property_read_u32(np, "samsung,armclk-cells", &cells))
> + return -EINVAL;
The property should be prefixed with "#" as other properties defining
number of cells.
Also you should check if cells value is correct, e.g. a number of cells
supported
> + prop = of_find_property(np, "samsung,armclk-divider-table", NULL);
> + if (!prop)
> + return -EINVAL;
> + if (!prop->value)
> + return -EINVAL;
> + if ((prop->length / sizeof(u32)) % cells)
> + return -EINVAL;
> + row = ((prop->length / sizeof(u32)) / cells) + 1;
Why + 1? Also the variable could be named in a bit more meaningful way,
e.g. num_rows.
> +
> + *data = kzalloc(sizeof(*tdata) * row, GFP_KERNEL);
> + if (!*data)
> + ret = -ENOMEM;
> + tdata = *data;
> +
> + val = prop->value;
> + for (; row > 1; row--, tdata++) {
> + for (col = 0; col < cells; col++)
> + cfg[col] = be32_to_cpup(val++);
You could use of_prop_next_u32() here.
> +
> + tdata->prate = cfg[0] * 1000;
> + tdata->div0 = EXYNOS4210_DIV_CPU0(cfg[6], cfg[5], cfg[4],
> + cfg[3], cfg[2], cfg[1]);
> + tdata->div1 = cells == 10 ?
> + EXYNOS4210_DIV_CPU1(cfg[9], cfg[8], cfg[7]) :
> + EXYNOS4210_DIV_CPU1(0, cfg[8], cfg[7]);
> + }
> + tdata->prate = 0;
> + return 0;
> +}
> +
> +static const struct samsung_cpuclk_soc_data exynos4210_cpuclk_soc_data = {
> + .parser = exynos4210_armclk_parser,
> + .ops = &exynos4210_armclk_clk_ops,
> + .offset = 0x14200,
> + .clk_cb = exynos4210_armclk_notifier_cb,
> +};
> +
> +static const struct samsung_cpuclk_soc_data exynos5250_cpuclk_soc_data = {
> + .parser = exynos4210_armclk_parser,
> + .ops = &exynos4210_armclk_clk_ops,
> + .offset = 0x200,
> + .clk_cb = exynos4210_armclk_notifier_cb,
> +};
> +
> +static const struct of_device_id samsung_clock_ids_armclk[] = {
> + { .compatible = "samsung,exynos4210-clock",
> + .data = &exynos4210_cpuclk_soc_data, },
> + { .compatible = "samsung,exynos4412-clock",
> + .data = &exynos4210_cpuclk_soc_data, },
> + { .compatible = "samsung,exynos5250-clock",
> + .data = &exynos5250_cpuclk_soc_data, },
> + { },
> +};
> +
> +/**
> + * samsung_register_arm_clock: register arm clock with ccf.
> + * @lookup_id: armclk clock output id for the clock controller.
> + * @parent: name of the parent clock for armclk.
> + * @base: base address of the clock controller from which armclk is generated.
> + * @np: device tree node pointer of the clock controller (optional).
> + * @ops: clock ops for this clock (optional)
> + */
> +int __init samsung_register_arm_clock(unsigned int lookup_id,
> + const char **parent_names, unsigned int num_parents,
> + void __iomem *base, struct device_node *np, struct clk_ops *ops)
> +{
> + const struct of_device_id *match;
> + const struct samsung_cpuclk_soc_data *data = NULL;
> +
> + if (np) {
> + match = of_match_node(samsung_clock_ids_armclk, np);
> + data = match ? match->data : NULL;
> + }
Since this is rather Exynos-specific and Exynos is DT-only, np being
NULL would be simply an error.
Best regards,
Tomasz
^ permalink raw reply
* [BUG] Circular locking dependency - DRM/CMA/MM/hotplug/...
From: Daniel Vetter @ 2014-02-12 18:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140212163317.GQ26684@n2100.arm.linux.org.uk>
On Wed, Feb 12, 2014 at 04:33:17PM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 12, 2014 at 04:40:50PM +0100, Marek Szyprowski wrote:
> >> -> #3 (console_lock){+.+.+.}:
> >> [<c0066f04>] __lock_acquire+0x151c/0x1ca0
> >> [<c0067c28>] lock_acquire+0xa0/0x130
> >> [<c006edcc>] console_lock+0x60/0x74
> >> [<c006f7b8>] console_cpu_notify+0x28/0x34
> >> [<c004904c>] notifier_call_chain+0x4c/0x8c
> >> [<c004916c>] __raw_notifier_call_chain+0x1c/0x24
> >> [<c0024124>] __cpu_notify+0x34/0x50
> >> [<c002424c>] cpu_notify_nofail+0x18/0x24
> >> [<c068e168>] _cpu_down+0x100/0x244
> >> [<c068e2dc>] cpu_down+0x30/0x44
> >> [<c036ef8c>] cpu_subsys_offline+0x14/0x18
> >> [<c036af28>] device_offline+0x94/0xbc
> >> [<c036b030>] online_store+0x4c/0x74
> >> [<c0368d3c>] dev_attr_store+0x20/0x2c
> >> [<c016b2e0>] sysfs_kf_write+0x54/0x58
> >> [<c016eaa4>] kernfs_fop_write+0xc4/0x160
> >> [<c0105a54>] vfs_write+0xbc/0x184
> >> [<c0105dfc>] SyS_write+0x48/0x70
> >> [<c000e6e0>] ret_fast_syscall+0x0/0x48
>
> cpu_down() takes cpu_hotplug.lock, so here we have:
>
> cpu_hotplug.lock
> console_lock
The patche I've linked in my other mail will break the chain here, so
should solve this. And apparently with cpu hotplug we can hit this, too.
And having banged my head against the console_lock wall I think doing a
trylock here is generally the sanest option.
So imo we can just blame console_lock, not need to either beat up v4l,
drm, cma or anyone else really ;-)
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* [RFC/PATCH 0/3] Add devicetree scanning for randomness
From: Jason Cooper @ 2014-02-12 18:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOesGMgNkdxW4znmns10-DPc4+OTWJLyx2fcJGTgdND6pp0zUQ@mail.gmail.com>
On Wed, Feb 12, 2014 at 10:13:59AM -0800, Olof Johansson wrote:
> On Wed, Feb 12, 2014 at 9:45 AM, Jason Cooper <jason@lakedaemon.net> wrote:
>
> > I brought this up at last weeks devicetree irc meeting. My goal is to
> > provide early randomness for kaslr on ARM. Currently, my idea is modify
> > the init script to save an additional random seed from /dev/urandom to
> > /boot/random-seed.
> >
> > The bootloader would then load this file into ram, and pass the
> > address/size to the kernel either via dt, or commandline. kaslr (run in
> > the decompressor) would consume some of this randomness, and then
> > random.c would consume the rest in a non-crediting initialization.
> >
> > While not ideal, it works in absence of an HRNG, and is no worse than
> > the current situation of storing the seed in /var/lib/misc/random-seed.
> > It also doesn't require modification of the bootloaders. Just an
> > updated kernel, and update the bootloader environment to load the
> > seed.
>
> Hmm. There are some drawbacks with this -- it assumes you can "just
> update the bootloader environment" which in general isn't easy to do.
true, the scope of my experience is consumer grade NASs, routers and
APs. At the very least, it's much easier than upgrading the bootloader.
Also, my pov is as a hobbyist modifying devices post-sale. The idea is
something I could add to my existing boxes without having to upgrade the
bootloaders.
> Also, you can't assume that /boot is writable or exists on all
> embedded systems.
In systems missing this capability, they often have the ability to
update the kernel. All that's needed is one block of flash. Current
random-seed size is 512 bytes. I'm not saying it's easy or desirable.
But for folks who feel it's necessary to have kaslr on embedded devices,
it would facilitate better random numbers. "Better" meaning much harder
for an attacker to guess.
> In general, taking both runtime and system-dependend data and using
> that to see entropy is a good idea.
Of course, I wasn't arguing for one or the other. As you said later, in
situations where you can't feed in a seed file, MAC addresses and serial
numbers are better than nothing.
thx,
Jason.
^ permalink raw reply
* [PATCH 1/3] mmc: add support for power-on sequencing through DT
From: Mark Brown @ 2014-02-12 18:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201401281148.10670.arnd@arndb.de>
On Tue, Jan 28, 2014 at 11:48:10AM +0100, Arnd Bergmann wrote:
> I think there is another option, which does have its own pros and cons:
> We could move all the power handling back into the sdio function driver
> if we allow a secondary detection path using DT rather than the probing
> of the SDIO bus. Essentially you'd have to list the class/vendor/device
> ID for each function that cannot be autodetected in DT, and have the
> SDIO core pretend that it found the device just by looking at the
> device nodes, and register the struct sdio_func so it can be bound to
> the driver. The driver then does all the power handling in a device
> specific way. At some point the hardware gets registered at the
> mmc host, and the sdio core connects the bus state to the already present
> sdio_func, possibly notifying the function driver that it has become
> usable.
> Obviously, this can only work for CAP_NONREMOVABLE devices, but those
> are exactly the ones we are worried about here. The advantage is that
> the power sequencing for a particular device can then be in device
> specific code and can have an arbitrarily complex in the driver without
> needing the mmc code to handle all possible corner cases.
I think this is going to be the most generic solution overall, it's
going to work with other buses and it's going to allow the device to
fully power itself off while running (I don't know if it's useful for SD
but that's definitely common with some other buses).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140212/aba45cd9/attachment.sig>
^ permalink raw reply
* [BUG] Circular locking dependency - DRM/CMA/MM/hotplug/...
From: Russell King - ARM Linux @ 2014-02-12 18:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140212182901.GX17001@phenom.ffwll.local>
On Wed, Feb 12, 2014 at 07:29:01PM +0100, Daniel Vetter wrote:
> On Wed, Feb 12, 2014 at 04:33:17PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 12, 2014 at 04:40:50PM +0100, Marek Szyprowski wrote:
> > >> -> #3 (console_lock){+.+.+.}:
> > >> [<c0066f04>] __lock_acquire+0x151c/0x1ca0
> > >> [<c0067c28>] lock_acquire+0xa0/0x130
> > >> [<c006edcc>] console_lock+0x60/0x74
> > >> [<c006f7b8>] console_cpu_notify+0x28/0x34
> > >> [<c004904c>] notifier_call_chain+0x4c/0x8c
> > >> [<c004916c>] __raw_notifier_call_chain+0x1c/0x24
> > >> [<c0024124>] __cpu_notify+0x34/0x50
> > >> [<c002424c>] cpu_notify_nofail+0x18/0x24
> > >> [<c068e168>] _cpu_down+0x100/0x244
> > >> [<c068e2dc>] cpu_down+0x30/0x44
> > >> [<c036ef8c>] cpu_subsys_offline+0x14/0x18
> > >> [<c036af28>] device_offline+0x94/0xbc
> > >> [<c036b030>] online_store+0x4c/0x74
> > >> [<c0368d3c>] dev_attr_store+0x20/0x2c
> > >> [<c016b2e0>] sysfs_kf_write+0x54/0x58
> > >> [<c016eaa4>] kernfs_fop_write+0xc4/0x160
> > >> [<c0105a54>] vfs_write+0xbc/0x184
> > >> [<c0105dfc>] SyS_write+0x48/0x70
> > >> [<c000e6e0>] ret_fast_syscall+0x0/0x48
> >
> > cpu_down() takes cpu_hotplug.lock, so here we have:
> >
> > cpu_hotplug.lock
> > console_lock
>
> The patche I've linked in my other mail will break the chain here, so
> should solve this. And apparently with cpu hotplug we can hit this, too.
> And having banged my head against the console_lock wall I think doing a
> trylock here is generally the sanest option.
>
> So imo we can just blame console_lock, not need to either beat up v4l,
> drm, cma or anyone else really ;-)
I don't think CMA needs to hold its lock across the allocations/frees
though - given the size of this, I think it's best if /everyone/ tries
to reduce the locking interactions where possible.
The CMA issue needs to be done anyway - what it currently means is that
all CMA allocations in the kernel are serialised, even if an allocation
attempt sleeps, another allocation gets blocked.
So, sorting that out breaks the dependency there, and if it can be broken
elsewhere, that's an added bonus and will help prevent other issues
like this.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* [RFC/PATCH 0/3] Add devicetree scanning for randomness
From: Jason Cooper @ 2014-02-12 18:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1571508.yGAAZ8TNH0@wuerfel>
On Wed, Feb 12, 2014 at 07:17:41PM +0100, Arnd Bergmann wrote:
> On Wednesday 12 February 2014 12:45:54 Jason Cooper wrote:
> > I brought this up at last weeks devicetree irc meeting. My goal is to
> > provide early randomness for kaslr on ARM. Currently, my idea is modify
> > the init script to save an additional random seed from /dev/urandom to
> > /boot/random-seed.
> >
> > The bootloader would then load this file into ram, and pass the
> > address/size to the kernel either via dt, or commandline. kaslr (run in
> > the decompressor) would consume some of this randomness, and then
> > random.c would consume the rest in a non-crediting initialization.
>
> I like the idea, but wouldn't it be easier to pass actual random data
> using DT, rather than the address/size?
I thought about that at first, but that requires either that the
bootloader be upgraded to insert the data, or that userspace is
modifying the dtb at least twice per boot.
I chose address/size to facilitate modifying existing/fielded devices.
The user could modify the dtb once, and modify the bootloader
environment to load X amount to Y address. As a fallback, it could be
expressed on the commandline for non-DT bootloaders.
So I'm not against the idea of random-seed,{start,size} and a
random-seed,blob. I would just like the former to be available for
folks interested in the capability on existing hardware w/o upgrading
the bootloader.
> That way we could even use the same DT binding for passing randomness
> from the bootloader, whereever it may have found that.
The problem lies in defining "whereever" ;)
> If the bootloader has internet connectivity, it could even mix in
> some data from http://www.random.org/cgi-bin/randbyte?nbytes=256&format=f
> ;-)
Gah! Arnd, you just about gave me a heart attack. And http no less.
thx,
Jason.
^ permalink raw reply
* [RFC/PATCH 0/3] Add devicetree scanning for randomness
From: Jason Cooper @ 2014-02-12 18:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140212182000.GJ5554@obsidianresearch.com>
On Wed, Feb 12, 2014 at 11:20:00AM -0700, Jason Gunthorpe wrote:
> On Wed, Feb 12, 2014 at 12:45:54PM -0500, Jason Cooper wrote:
>
> > The bootloader would then load this file into ram, and pass the
> > address/size to the kernel either via dt, or commandline. kaslr (run in
> > the decompressor) would consume some of this randomness, and then
> > random.c would consume the rest in a non-crediting initialization.
>
> Sure is a neat idea, but I think in general it would probably be smart
> to include the entire FDT blob in the early random pool, that way you
> get MACs and other machine unique data too.
Sure.
> From there it is a small step to encourage bootloaders to include
> boot-time-variable data in the DT like like 'boot time of day', 'cycle
> counter', 'random blob', etc.
I like it.
> Then you just need the bootloader to dump the random-seed file into a
> DT property.
Yes, see my response to Arnd re the binding. I'm also interested in
making it easier for devices already in the field. iow, without
upgrading the bootloader.
> Or have the bootloader fetch randomness from any HWRNG it has a driver
> for. (eg a TPM)
Depends on who you're protecting against. I'd prefer to have that
called out as a separate blob in the DT so the kernel could decide
whether to trust it explicitly, or mix it like random.c already does
with RDRAND.
thx,
Jason.
^ permalink raw reply
* [PATCH 2/3] arm64: Add Kconfig option for Samsung GH7 SoC family
From: Kumar Gala @ 2014-02-12 19:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1540B9A3-2485-4149-81DD-7D86A532E5D4@arm.com>
On Feb 12, 2014, at 12:12 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On 12 Feb 2014, at 16:25, Kumar Gala <galak@codeaurora.org> wrote:
>> On Feb 12, 2014, at 4:38 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>>
>>> On Tue, Feb 11, 2014 at 11:39:27PM +0000, Olof Johansson wrote:
>>>> On Mon, Feb 10, 2014 at 10:29 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
>>>>> This patch adds support for Samsung GH7 SoC in arm64/Kconfig.
>>>>>
>>>>> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
>>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>>
>>>> The overhead of building one more device tree isn't very large, and I
>>>> don't see any other need to have a Kconfig entry per SoC at this time.
>>>> It's of course up to Catalin, but you might just want to always
>>>> compile all dts files instead.
>>>
>>> For arm64, I thought of getting rid of ARCH_* Kconfig entries entirely,
>>> only that I haven't heard any strong opinion either way (in which case
>>> I'll do it, with a risk of single Image getting bigger and bigger and
>>> people needing smaller Image can trim their .config).
>>
>> One reason to keep around ARCH_* is for drivers shared between arm and arm64 that depend on it.
>
> We already converted some of them (those depending on ARCH_VEXPRESS) to
> just depend on ARM64. Ideally, at some point I?d like to see them as
> defaulting to modules but I don?t think we are there yet (we had some
> discussions at the last KS, I?m not sure anyone started looking into
> this).
I?m torn about this, I think for something like VEXPRESS it makes sense, however I think its reasonable to still have an config symbol for a full SoC family or something of that nature.
- k
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* [PATCH 1/2] ARM: tegra: enable I2C Mux driver for PCA9546 in defconfig
From: Stephen Warren @ 2014-02-12 19:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1391817298-10600-1-git-send-email-pengw@nvidia.com>
On 02/07/2014 04:54 PM, Bryan Wu wrote:
> PCA9546 is used in Cardhu Tegra30 board to connect to 3 cameras.
> Enabling this driver for Tegra V4L2 soc camera driver and camera
> sensor drivers.
I've squashed patch 1/2 into Tegra's for-3.15/defconfig branch, and
applied patch 2/2 to Tegra's for-3.15/dt branch.
Note that your patches were sent with an email "From" address that
didn't match your signed-off-by tag, yet "git send-email" didn't insert
a "From" header in the email body. Can you please check your git user ID
and/or email settings. Consequently, I had to adjust the git author
field in git to match your s-o-b line.
^ permalink raw reply
* [RFC/PATCH 0/3] Add devicetree scanning for randomness
From: Arnd Bergmann @ 2014-02-12 19:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140212184521.GO27395@titan.lakedaemon.net>
On Wednesday 12 February 2014 13:45:21 Jason Cooper wrote:
> On Wed, Feb 12, 2014 at 07:17:41PM +0100, Arnd Bergmann wrote:
> > On Wednesday 12 February 2014 12:45:54 Jason Cooper wrote:
> > > I brought this up at last weeks devicetree irc meeting. My goal is to
> > > provide early randomness for kaslr on ARM. Currently, my idea is modify
> > > the init script to save an additional random seed from /dev/urandom to
> > > /boot/random-seed.
> > >
> > > The bootloader would then load this file into ram, and pass the
> > > address/size to the kernel either via dt, or commandline. kaslr (run in
> > > the decompressor) would consume some of this randomness, and then
> > > random.c would consume the rest in a non-crediting initialization.
> >
> > I like the idea, but wouldn't it be easier to pass actual random data
> > using DT, rather than the address/size?
>
> I thought about that at first, but that requires either that the
> bootloader be upgraded to insert the data, or that userspace is
> modifying the dtb at least twice per boot.
>
> I chose address/size to facilitate modifying existing/fielded devices.
> The user could modify the dtb once, and modify the bootloader
> environment to load X amount to Y address. As a fallback, it could be
> expressed on the commandline for non-DT bootloaders.
Ah, so you are interested in boot loaders that can be scripted to do
what you had in mind but cannot be scripted to add or modify a DT
property. I hadn't considered that, but you are probably right that
this is at least 90% of the systems you'd find in the wild today.
Thinking this a bit further, I wonder if (at least upstream) u-boot
has a way to modify DT properties in a scripted way that would allow
the direct property. It sounds like a generally useful feature not
just for randomness, so if that doesn't already work, maybe someone
can implement it. In the simplest case, you'd only need to find the
address of an existing property in the dtb and load a file to
that location.
Arnd
^ permalink raw reply
* [PATCH 2/3] arm64: Add Kconfig option for Samsung GH7 SoC family
From: Arnd Bergmann @ 2014-02-12 19:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <D225ECFF-E2A7-44C9-8D11-20EE77077797@codeaurora.org>
On Wednesday 12 February 2014 13:04:40 Kumar Gala wrote:
> On Feb 12, 2014, at 12:12 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On 12 Feb 2014, at 16:25, Kumar Gala <galak@codeaurora.org> wrote:
> >> One reason to keep around ARCH_* is for drivers shared between arm and arm64 that depend on it.
> >
> > We already converted some of them (those depending on ARCH_VEXPRESS) to
> > just depend on ARM64. Ideally, at some point I?d like to see them as
> > defaulting to modules but I don?t think we are there yet (we had some
> > discussions at the last KS, I?m not sure anyone started looking into
> > this).
>
> I?m torn about this, I think for something like VEXPRESS it makes sense,
> however I think its reasonable to still have an config symbol for a full
> SoC family or something of that nature.
I think for SBSA compliant systems, we should be able to live with a
generic ARCH_SBSA Kconfig symbol. For more irregular embedded platforms,
we may need something more specific.
Arnd
^ permalink raw reply
* [PATCH] ARM: tegra: don't timeout if CPU is powergated
From: Stephen Warren @ 2014-02-12 19:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1d5ea6b7df4573d866779857922ac650fe59af60.1392078805.git.stefan@agner.ch>
On 02/10/2014 05:44 PM, Stefan Agner wrote:
> When booting secondary CPU(s) which are not yet powergated, a wrong
> check lead to a timeout after 100 jiffies. With this patch, we only
> delay powergating if CPUs are still not powered yet.
I've applied this to Tegra's for-3.15/soc branch.
^ permalink raw reply
* [PATCH] pinctrl: sunxi: use chained_irq_{enter, exit} for GIC compatibility
From: Maxime Ripard @ 2014-02-12 19:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1392049357-13225-1-git-send-email-wens@csie.org>
On Tue, Feb 11, 2014 at 12:22:37AM +0800, Chen-Yu Tsai wrote:
> On tha Allwinner A20 SoC, the external interrupts on the pin controller
> device are connected to the GIC. Without chained_irq_{enter, exit},
> external GPIO interrupts, such as used by mmc core card detect, cause
> the system to hang.
>
> Cc: stable at vger.kernel.org
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140212/56433c62/attachment.sig>
^ permalink raw reply
* [PATCH] ARM: tegra: dalmore: fix irq trigger type
From: Stephen Warren @ 2014-02-12 19:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <30f0db808e7543d9ab6b077aa5e239f2@agner.ch>
On 02/11/2014 02:21 PM, Stefan Agner wrote:
> Am 2014-02-11 21:47, schrieb Thierry Reding:
>> On Tue, Feb 11, 2014 at 09:11:32PM +0100, Stefan Agner wrote:
>>> Trigger type needs to be IRQ_TYPE_LEVEL_HIGH since the interrupt
>>> signal gets inverted by the PMC (configured by the invert-interrupt
>>> property).
>>
>> Isn't the reason the other way around? The PMIC generates a low-level
>> interrupt, but the GIC can only be configured to accept high-level (or
>> rising edge) and therefore the nvidia,invert-interrupt property needs to
>> be set in the PMC node?
> Hm yes agreed. I should also write the whole story here, maybe this:
>
> The GIC only support high-active interrupts. When using a PMIC with
> low-active interrupt, the PMC has to be configured by using the
> nvidia,invert-interrupt property in its node.
>
> This fix sets the GIC back to high-active and reverts commit
> eca8f98e404934027f84f72882c5e92ffbd9e5f5.
(Trimming CC lists)
Stefan,
It'd be best to include the commit subject rather than just the commit
hash, i.e.:
... and reverts commit eca8f98e4049 "ARM: tegra: dalmore: fix the irq
trigger type of Palmas MFD device".
It may also be helpful for the commit description to quote the kernel
boot message which this patch solves:
> [ 0.215178] genirq: Setting trigger mode 8 for irq 118 failed (gic_set_type+0x0/0xf4)
For me, applying this patch actually *causes* an interrupt storm, rather
than preventing one. Yet without it, no interrupts occur at all. I
wonder if the driver has a bug where it's not correctly clearing all
interrupt status (e.g. something pre-existing before boot), so once the
polarity is set up correctly, the interrupt is stuck?
Joseph,
As the author of the patch that's being reverted, can you please comment
here?
^ permalink raw reply
* [PATCH 2/3] PCI: ARM: add support for virtual PCI host controller
From: Jason Gunthorpe @ 2014-02-12 19:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2006726.HhIT01YuXY@wuerfel>
On Tue, Feb 11, 2014 at 11:42:52AM +0100, Arnd Bergmann wrote:
> I looked briefly at the code and found that mach-kirkwood/pcie.c does
> both request_resource() and pci_add_resource_offset(), while
> drivers/pci/host/pci-mvebu.c only does the latter. Does the patch
> below restore the previous behavior?
It gets closer:
e0000000-f0000000 : <BAD>
e0000000-e00fffff : PCI Bus 0000:01
e0000000-e001ffff : 0000:01:00.0
e0001000-e0001fff : /mbus/pex at e0000000/pcie at 1,0/fpga at 0/fpga_sysmon at 1000
This patch:
diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
index 2394e97..7fd54e9 100644
--- a/drivers/bus/mvebu-mbus.c
+++ b/drivers/bus/mvebu-mbus.c
@@ -876,14 +876,14 @@ static void __init mvebu_mbus_get_pcie_resources(struct device_node *np,
ret = of_property_read_u32_array(np, "pcie-mem-aperture", reg, ARRAY_SIZE(reg));
if (!ret) {
mem->start = reg[0];
- mem->end = mem->start + reg[1];
+ mem->end = mem->start + reg[1] - 1;
mem->flags = IORESOURCE_MEM;
}
ret = of_property_read_u32_array(np, "pcie-io-aperture", reg, ARRAY_SIZE(reg));
if (!ret) {
io->start = reg[0];
- io->end = io->start + reg[1];
+ io->end = io->start + reg[1] - 1;
io->flags = IORESOURCE_IO;
}
}
Fixes the wrong length (e0000000-f0000000 should be e0000000-efffffff)
And this fixes the <BAD>:
diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index ef8691a..fbb89cb 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -109,7 +109,9 @@ struct mvebu_pcie {
struct mvebu_pcie_port *ports;
struct msi_chip *msi;
struct resource io;
+ char io_name[30];
struct resource realio;
+ char mem_name[30];
struct resource mem;
struct resource busn;
int nports;
@@ -681,10 +683,29 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys)
{
struct mvebu_pcie *pcie = sys_to_pcie(sys);
int i;
+ int domain = 0;
+#ifdef CONFIG_PCI_DOMAINS
+ domain = sys->domain;
+#endif
+
+ snprintf(pcie->mem_name, sizeof(pcie->mem_name), "PCI MEM %04x", domain);
+ pcie->mem.name = pcie->mem_name;
+
+ snprintf(pcie->io_name, sizeof(pcie->io_name), "PCI I/O %04x", domain);
+ pcie->realio.name = pcie->io_name;
Still missing release_region..
Thoughts on upstreamining these bits?
> Since the mvebu_pcie_setup() function seems very generic at this,
> we should probably try to factor out that code into a common
> helper, at least for arm64, but ideally shared with arm32
> as well.
Yah, especially since people are not getting it completely right..
But some of the trouble here is a lack of a generic pci host driver
structure, eg I have to pull the domain number out of the ARM32
specific structure ..
Jason
^ permalink raw reply related
* [RFC/PATCH 0/3] Add devicetree scanning for randomness
From: Jason Cooper @ 2014-02-12 19:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1882539.R1gpoLLYks@wuerfel>
On Wed, Feb 12, 2014 at 08:12:23PM +0100, Arnd Bergmann wrote:
> On Wednesday 12 February 2014 13:45:21 Jason Cooper wrote:
> > On Wed, Feb 12, 2014 at 07:17:41PM +0100, Arnd Bergmann wrote:
> > > On Wednesday 12 February 2014 12:45:54 Jason Cooper wrote:
> > > > I brought this up at last weeks devicetree irc meeting. My goal is to
> > > > provide early randomness for kaslr on ARM. Currently, my idea is modify
> > > > the init script to save an additional random seed from /dev/urandom to
> > > > /boot/random-seed.
> > > >
> > > > The bootloader would then load this file into ram, and pass the
> > > > address/size to the kernel either via dt, or commandline. kaslr (run in
> > > > the decompressor) would consume some of this randomness, and then
> > > > random.c would consume the rest in a non-crediting initialization.
> > >
> > > I like the idea, but wouldn't it be easier to pass actual random data
> > > using DT, rather than the address/size?
> >
> > I thought about that at first, but that requires either that the
> > bootloader be upgraded to insert the data, or that userspace is
> > modifying the dtb at least twice per boot.
> >
> > I chose address/size to facilitate modifying existing/fielded devices.
> > The user could modify the dtb once, and modify the bootloader
> > environment to load X amount to Y address. As a fallback, it could be
> > expressed on the commandline for non-DT bootloaders.
>
> Ah, so you are interested in boot loaders that can be scripted to do
> what you had in mind but cannot be scripted to add or modify a DT
> property. I hadn't considered that, but you are probably right that
> this is at least 90% of the systems you'd find in the wild today.
Yes, exactly.
> Thinking this a bit further, I wonder if (at least upstream) u-boot
> has a way to modify DT properties in a scripted way that would allow
> the direct property. It sounds like a generally useful feature not
> just for randomness, so if that doesn't already work, maybe someone
> can implement it. In the simplest case, you'd only need to find the
> address of an existing property in the dtb and load a file to
> that location.
Like fdtget from dtc? The thing is, this address would only need to be
setup once per board. Hypothetically, debian's flash-kernel could set
the address in the dtb, then set it in the u-boot environment. From
then on, only the normal initscript writing to random-seed would be
needed.
thx,
Jason.
^ 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