* [PATCH V2 2/5] arm64: Allow hw watchpoint at varied offset from base address
From: Pratyush Anand @ 2016-11-09 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108032658.GB20591@arm.com>
Hi Will,
Thanks a lot for your review.
On Tuesday 08 November 2016 08:56 AM, Will Deacon wrote:
> Hi Pratyush,
>
> On Thu, Oct 20, 2016 at 11:18:14AM +0530, Pratyush Anand wrote:
>> ARM64 hardware supports watchpoint at any double word aligned address.
>> However, it can select any consecutive bytes from offset 0 to 7 from that
>> base address. For example, if base address is programmed as 0x420030 and
>> byte select is 0x1C, then access of 0x420032,0x420033 and 0x420034 will
>> generate a watchpoint exception.
>>
>> Currently, we do not have such modularity. We can only program byte,
>> halfword, word and double word access exception from any base address.
>>
>> This patch adds support to overcome above limitations.
>>
>> Signed-off-by: Pratyush Anand <panand@redhat.com>
>> ---
>> arch/arm64/include/asm/hw_breakpoint.h | 2 +-
>> arch/arm64/kernel/hw_breakpoint.c | 45 ++++++++++++++++------------------
>> arch/arm64/kernel/ptrace.c | 5 ++--
>> 3 files changed, 25 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
>> index 115ea2a64520..4f4e58bee9bc 100644
>> --- a/arch/arm64/include/asm/hw_breakpoint.h
>> +++ b/arch/arm64/include/asm/hw_breakpoint.h
>> @@ -118,7 +118,7 @@ struct perf_event;
>> struct pmu;
>>
>> extern int arch_bp_generic_fields(struct arch_hw_breakpoint_ctrl ctrl,
>> - int *gen_len, int *gen_type);
>> + int *gen_len, int *gen_type, int *offset);
>> extern int arch_check_bp_in_kernelspace(struct perf_event *bp);
>> extern int arch_validate_hwbkpt_settings(struct perf_event *bp);
>> extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
>> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
>> index 26a6bf77d272..3c2b96803eba 100644
>> --- a/arch/arm64/kernel/hw_breakpoint.c
>> +++ b/arch/arm64/kernel/hw_breakpoint.c
>> @@ -349,7 +349,7 @@ int arch_check_bp_in_kernelspace(struct perf_event *bp)
>> * to generic breakpoint descriptions.
>> */
>> int arch_bp_generic_fields(struct arch_hw_breakpoint_ctrl ctrl,
>> - int *gen_len, int *gen_type)
>> + int *gen_len, int *gen_type, int *offset)
>> {
>> /* Type */
>> switch (ctrl.type) {
>> @@ -369,8 +369,10 @@ int arch_bp_generic_fields(struct arch_hw_breakpoint_ctrl ctrl,
>> return -EINVAL;
>> }
>>
>> + *offset = ffs(ctrl.len) - 1;
>
> Do we want to fail the length == 0 case early? If you do add that check,
> then you can use __ffs here instead.
Yes, I think, your point can be taken.
>
>> /* Len */
>> - switch (ctrl.len) {
>> + switch (ctrl.len >> *offset) {
>> case ARM_BREAKPOINT_LEN_1:
>> *gen_len = HW_BREAKPOINT_LEN_1;
>> break;
>> @@ -517,18 +519,17 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
>> default:
>> return -EINVAL;
>> }
>> -
>> - info->address &= ~alignment_mask;
>> - info->ctrl.len <<= offset;
>> } else {
>> if (info->ctrl.type == ARM_BREAKPOINT_EXECUTE)
>> alignment_mask = 0x3;
>> else
>> alignment_mask = 0x7;
>> - if (info->address & alignment_mask)
>> - return -EINVAL;
>> + offset = info->address & alignment_mask;
>> }
>>
>> + info->address &= ~alignment_mask;
>> + info->ctrl.len <<= offset;
>
> Urgh, I really hate all this converting to and fro to keep perf happy.
> FWIW, I'm at the point where I would seriously consider ripping out the
> hw_breakpoint code and replacing it with a ptrace-specific backend so we
> just drop all this crap. The only people that seem to use the perf interface
> are those running the (failing) selftests. Given that we have to have
> a bloody thread switch notifier *anyway*, all perf seems to give us is
> complexity and restrictions.
Yes, I agree.
>
>> /*
>> * Disallow per-task kernel breakpoints since these would
>> * complicate the stepping code.
>> @@ -665,8 +666,8 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
>> struct pt_regs *regs)
>> {
>> int i, step = 0, *kernel_step, access;
>> - u32 ctrl_reg;
>> - u64 val, alignment_mask;
>> + u32 ctrl_reg, lens, lene;
>> + u64 val;
>> struct perf_event *wp, **slots;
>> struct debug_info *debug_info;
>> struct arch_hw_breakpoint *info;
>> @@ -684,25 +685,21 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
>> goto unlock;
>>
>> info = counter_arch_bp(wp);
>> - /* AArch32 watchpoints are either 4 or 8 bytes aligned. */
>> - if (is_compat_task()) {
>> - if (info->ctrl.len == ARM_BREAKPOINT_LEN_8)
>> - alignment_mask = 0x7;
>> - else
>> - alignment_mask = 0x3;
>> - } else {
>> - alignment_mask = 0x7;
>> - }
>>
>> - /* Check if the watchpoint value matches. */
>> + /* Check if the watchpoint value and byte select match. */
>> val = read_wb_reg(AARCH64_DBG_REG_WVR, i);
>> - if (val != (addr & ~alignment_mask))
>> - goto unlock;
>> -
>> - /* Possible match, check the byte address select to confirm. */
>> ctrl_reg = read_wb_reg(AARCH64_DBG_REG_WCR, i);
>> decode_ctrl_reg(ctrl_reg, &ctrl);
>> - if (!((1 << (addr & alignment_mask)) & ctrl.len))
>> + lens = ffs(ctrl.len) - 1;
>> + lene = fls(ctrl.len) - 1;
>
> Again, you can use the '__' variants to avoid the '- 1'.
Ok.
>
>> + /*
>> + * FIXME: reported address can be anywhere between "the
>> + * lowest address accessed by the memory access that
>> + * triggered the watchpoint" and "the highest watchpointed
>> + * address accessed by the memory access". So, it may not
>> + * lie in the interval of watchpoint address range.
>> + */
>> + if (addr < val + lens || addr > val + lene)
>> goto unlock;
>>
>> /*
>> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
>> index e0c81da60f76..0eb366a94382 100644
>> --- a/arch/arm64/kernel/ptrace.c
>> +++ b/arch/arm64/kernel/ptrace.c
>> @@ -327,13 +327,13 @@ static int ptrace_hbp_fill_attr_ctrl(unsigned int note_type,
>> struct arch_hw_breakpoint_ctrl ctrl,
>> struct perf_event_attr *attr)
>> {
>> - int err, len, type, disabled = !ctrl.enabled;
>> + int err, len, type, offset, disabled = !ctrl.enabled;
>>
>> attr->disabled = disabled;
>> if (disabled)
>> return 0;
>>
>> - err = arch_bp_generic_fields(ctrl, &len, &type);
>> + err = arch_bp_generic_fields(ctrl, &len, &type, &offset);
>> if (err)
>> return err;
>>
>> @@ -352,6 +352,7 @@ static int ptrace_hbp_fill_attr_ctrl(unsigned int note_type,
>>
>> attr->bp_len = len;
>> attr->bp_type = type;
>> + attr->bp_addr += offset;
>
> That's going to look pretty bizarre from userspace if it decides to read
> back the address registers to find that they've mysteriously changed.
>
> Perhaps ptrace_hbp_get_addr needs to retrieve the address from the
> arch_hw_breakpoint, like we do for the ctrl register. What do you think?
..and that should help...I will give it a try and will test before next
revision.
~Pratyush
^ permalink raw reply
* [PATCH V2 3/5] arm64: hw_breakpoint: Handle inexact watchpoint addresses
From: Pratyush Anand @ 2016-11-09 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJt8pk9tH5nm5sUsKx-dAvW4uzKP+EoRumhYV1mXsU_mk6dRDg@mail.gmail.com>
On Tuesday 08 November 2016 05:28 PM, Pavel Labath wrote:
>>> + if (min_dist > 0 && min_dist != -1) {
>>> >> + /* No exact match found. */
>>> >> + wp = slots[closest_match];
>>> >> + info = counter_arch_bp(wp);
>>> >> + info->trigger = addr;
>>> >> + perf_bp_event(wp, regs);
>>> >> + }
>> >
>> > Why don't we need to bother with the stepping in the case of a non-exact
>> > match?
> Good catch. I think we do. I must have dropped that part somehow.
>
> Pratyush, could you include the attached fixup in the next batch?
Ok, will do.
~Pratyush
^ permalink raw reply
* [PATCH 1/5] iommu: Allow taking a reference on a group directly
From: Robin Murphy @ 2016-11-09 17:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161109172543.GI17771@arm.com>
On 09/11/16 17:25, Will Deacon wrote:
> On Wed, Nov 09, 2016 at 12:47:24PM +0000, Robin Murphy wrote:
>> iommu_group_get_for_dev() expects that the IOMMU driver's device_group
>> callback return a group with a reference held for the given device.
>> Whilst allocating a new group is fine, and pci_device_group() correctly
>> handles reusing an existing group, there is no general means for IOMMU
>> drivers doing their own group lookup to take additional references on an
>> existing group pointer without having to also store device pointers or
>> resort to elaborate trickery.
>>
>> Add an IOMMU-driver-specific function to fill the hole.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>> drivers/iommu/iommu.c | 14 ++++++++++++++
>> include/linux/iommu.h | 1 +
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 9a2f1960873b..b0b052bc6bb5 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -552,6 +552,20 @@ struct iommu_group *iommu_group_get(struct device *dev)
>> EXPORT_SYMBOL_GPL(iommu_group_get);
>>
>> /**
>> + * __iommu_group_get - Increment reference on a group
>> + * @group: the group to use, must not be NULL
>> + *
>> + * This function may be called by internal iommu driver group management
>> + * when the context of a struct device pointer is not available. It is
>> + * not for general use. Returns the given group for convenience.
>> + */
>> +struct iommu_group *__iommu_group_get(struct iommu_group *group)
>> +{
>> + kobject_get(group->devices_kobj);
>> + return group;
>> +}
>
> This probably either wants sticking in a header or exporting to modules.
> That said, why do we need the underscores and the comment about internal
> group management? That's pretty much already the case for iommu_group_get.
The definition of struct iommu_group is private to iommu.c, so any
touching of the members has to be in here. The comment is to contrast
with iommu_group_get()'s "This function is called by iommu drivers and
users". This one is explicitly not for users of the API (DMA mapping,
VFIO, etc.), as they really have no business messing with refcounts
directly, and should always be operating in the context of a device;
it's only for the benefit of anyone *implementing* the API. And since
IOMMU drivers aren't modular (yet... ;)) there's no cause for an export.
> Of course, removing the underscores gives you a naming conflict, but we
> could just call it something like "iommu_group_get_ref".
Ideally, this would be the iommu_group_get() to precisely match
iommu_group_put(), and the existing function would renamed something
like iommu_dev_group_get() (or perhaps even all external uses converted
over to iommu_group_get_for_dev()), but that would be an awful lot of
churn for little obvious benefit. Similarly, I nearly added the below
hunk, but it didn't seem worth the bother.
Robin.
----->8-----
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 9a2f1960873b..89d509c59019 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -545,7 +545,7 @@ struct iommu_group *iommu_group_get(struct device *dev)
struct iommu_group *group = dev->iommu_group;
if (group)
- kobject_get(group->devices_kobj);
+ __iommu_group_get(group);
return group;
}
^ permalink raw reply related
* [PATCH net-next 0/2] ARM64: Add Internal PHY support for Meson GXL
From: David Miller @ 2016-11-09 17:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478274683-1503-1-git-send-email-narmstrong@baylibre.com>
From: Neil Armstrong <narmstrong@baylibre.com>
Date: Fri, 4 Nov 2016 16:51:21 +0100
> The Amlogic Meson GXL SoCs have an internal RMII PHY that is muxed with the
> external RGMII pins.
>
> In order to support switching between the two PHYs links, extended registers
> size for mdio-mux-mmioreg must be added.
>
> The DT related patches submitted as RFC in [3] will be sent in a separate
> patchset due to multiple patchsets and DTSI migrations.
>
> Changes since v2 RFC patchset at : [3]
> - Change phy Kconfig/Makefile alphabetic order
> - GXL dtsi cleanup
>
> Changes since original RFC patchset at : [2]
> - Remove meson8b experimental phy switching
> - Switch to mdio-mux-mmioreg with extennded size support
> - Add internal phy support for S905x and p231
> - Add external PHY support for p230
>
> [1] http://lkml.kernel.org/r/1477932286-27482-1-git-send-email-narmstrong at baylibre.com
> [2] http://lkml.kernel.org/r/1477060838-14164-1-git-send-email-narmstrong at baylibre.com
> [3] http://lkml.kernel.org/r/1477932987-27871-1-git-send-email-narmstrong at baylibre.com
Series applied, thanks.
^ permalink raw reply
* [PATCH v7] soc: qcom: add l2 cache perf events driver
From: Mark Rutland @ 2016-11-09 17:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477687813-11412-1-git-send-email-nleeder@codeaurora.org>
Hi Neil,
Apologies for the delay in replying to this.
This is looking good. I have a few specific comments and a couple of
general concerns below.
Will, please see the bit below about cluster/socket aggregation (grep
for your name).
On Fri, Oct 28, 2016 at 04:50:13PM -0400, Neil Leeder wrote:
> +#include <linux/acpi.h>
> +#include <linux/interrupt.h>
> +#include <linux/perf_event.h>
> +#include <linux/platform_device.h>
While scanning the driver. I noticed a number of other headers that are
required for things the driver explicitly references. Please add the
following so that we're not relying on (fragile) transitive includes:
#include <linux/bitops.h>
#include <linux/bug.h>
#include <linux/cpuhotplug.h>
#include <linux/cpumask.h>
#include <linux/device.h>
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/percpu.h>
#include <linux/smp.h>
#include <linux/spinlock.h>
#include <linux/sysfs.h>
#include <linux/types.h>
#include <asm/barrier.h>
#include <asm/local64.h>
#include <asm/sysreg.h>
[...]
> +/*
> + * Aggregate PMU. Implements the core pmu functions and manages
> + * the hardware PMUs.
> + */
> +struct l2cache_pmu {
> + struct hlist_node node;
> + u32 num_pmus;
> + struct pmu pmu;
> + int num_counters;
> + cpumask_t cpumask;
> + struct platform_device *pdev;
> +};
> +
> +/*
> + * The cache is made up of one or more clusters, each cluster has its own PMU.
> + * Each cluster is associated with one or more CPUs.
> + * This structure represents one of the hardware PMUs.
> + *
> + * Events can be envisioned as a 2-dimensional array. Each column represents
> + * a group of events. There are 8 groups. Only one entry from each
> + * group can be in use at a time. When an event is assigned a counter
> + * by *_event_add(), the counter index is assigned to group_to_counter[group].
If I've followed correctly, this means each group has a dedicated
counter?
I take it groups are not symmetric (i.e. each column has different
events)? Or does each column contain the same events?
Is there any overlap?
> + * This allows *filter_match() to detect and reject conflicting events in
> + * the same group.
> + * Events are specified as 0xCCG, where CC is 2 hex digits specifying
> + * the code (array row) and G specifies the group (column).
> + *
> + * In addition there is a cycle counter event specified by L2CYCLE_CTR_RAW_CODE
> + * which is outside the above scheme.
> + */
> +struct hml2_pmu {
It's not clear to me what "hml2" is, and now we seem to use "cluster"
and "hml2" interchangeably in comments and code. Can you please define
what "hml2" is in a comment, mentioning that we refer to this (or its
container?) as a cluster?
Can we also please s/hml2/cluster/ in the code, for consistency?
> + struct perf_event *events[MAX_L2_CTRS];
> + struct l2cache_pmu *l2cache_pmu;
> + DECLARE_BITMAP(used_counters, MAX_L2_CTRS);
> + DECLARE_BITMAP(used_groups, L2_EVT_GROUP_MAX + 1);
> + int group_to_counter[L2_EVT_GROUP_MAX + 1];
> + int irq;
> + /* The CPU that is used for collecting events on this cluster */
> + int on_cpu;
> + /* All the CPUs associated with this cluster */
> + cpumask_t cluster_cpus;
I'm still uncertain about aggregating all cluster PMUs into a larger
PMU, given the cluster PMUs are logically independent (at least in terms
of the programming model).
However, from what I understand the x86 uncore PMU drivers aggregate
symmetric instances of uncore PMUs (and also aggregate across packages
to the same logical PMU).
Whatever we do, it would be nice for the uncore drivers to align on a
common behaviour (and I think we're currently going the oppposite route
with Cavium's uncore PMU). Will, thoughts?
> + spinlock_t pmu_lock;
> +};
[...]
> +static void hml2_pmu__set_resr(struct hml2_pmu *cluster,
> + u32 event_group, u32 event_cc)
> +{
> + u64 field;
> + u64 resr_val;
> + u32 shift;
> + unsigned long flags;
> +
> + shift = L2PMRESR_GROUP_BITS * event_group;
> + field = ((u64)(event_cc & L2PMRESR_GROUP_MASK) << shift) | L2PMRESR_EN;
The L2PMRESR_EN isn't part of the shifted field...
> +
> + spin_lock_irqsave(&cluster->pmu_lock, flags);
> +
> + resr_val = get_l2_indirect_reg(L2PMRESR);
> + resr_val &= ~(L2PMRESR_GROUP_MASK << shift);
> + resr_val |= field;
... so can we please or that in on a separate line here, e.g.
resr_val |= L2PMRESR_EN;
That said, do we need to set this bit each time we program an event?
> + set_l2_indirect_reg(L2PMRESR, resr_val);
> +
> + spin_unlock_irqrestore(&cluster->pmu_lock, flags);
> +}
[...]
> +static void l2_cache__event_update_from_cluster(struct perf_event *event,
> + struct hml2_pmu *cluster)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + u64 delta64, prev, now;
> + u32 delta;
> + u32 idx = hwc->idx;
> +
> + do {
> + prev = local64_read(&hwc->prev_count);
> + now = hml2_pmu__counter_get_value(idx);
> + } while (local64_cmpxchg(&hwc->prev_count, prev, now) != prev);
> +
> + if (idx == l2_cycle_ctr_idx) {
> + /*
> + * The cycle counter is 64-bit so needs separate handling
> + * of 64-bit delta.
> + */
> + delta64 = now - prev;
> + local64_add(delta64, &event->count);
> + } else {
> + /*
> + * 32-bit counters need the unsigned 32-bit math to handle
> + * overflow and now < prev
> + */
> + delta = now - prev;
> + local64_add(delta, &event->count);
> + }
> +}
I believe you can get rid of the delta/delta64 split with something like
the below (with a 64-bit delta):
/*
* The cycle counter is 64-bit, but all other counters are
* 32-bit, and we must handle 32-bit overflow explicitly.
*/
delta = now - prev;
if (idx != l2_cycle_ctr_idx)
delta &= 0xffffffff;
local64_add(delta, &event->count);
> +
> +static void l2_cache__cluster_set_period(struct hml2_pmu *cluster,
> + struct hw_perf_event *hwc)
> +{
> + u64 base = L2_MAX_PERIOD - (L2_CNT_PERIOD - 1);
Is L2_CNT_PERIOD a hw-derived value? Or just something that happened to
be small enough to avoid overflow in testing?
> + u32 idx = hwc->idx;
> + u64 prev = local64_read(&hwc->prev_count);
> + u64 value;
> +
> + /*
> + * Limit the maximum period to prevent the counter value
> + * from overtaking the one we are about to program.
> + * Use a starting value which is high enough that after
> + * an overflow, interrupt latency will not cause the count
> + * to reach the base value. If the previous value
> + * is below the base, increase it to be above the base
> + * and update prev_count accordingly. Otherwise if
> + * the previous value is already above the base
> + * nothing needs to be done to prev_count.
> + */
> + if (prev < base) {
> + value = base + prev;
> + local64_set(&hwc->prev_count, value);
> + } else {
> + value = prev;
> + }
> +
> + hml2_pmu__counter_set_value(idx, value);
> +}
Given that we don't do sampling, and are only using the interrupt to
prevent losing events in the case of overflow, I think we can use a
constant value to handle all cases.
Additionally, I think we need to handle the cycle counter separately,
since it has a different width. Something like the below (perhaps with
different constants):
static void l2_cache__cluster_set_period(struct hml2_pmu *cluster,
struct hw_perf_event *hwc)
{
u64 new;
/*
* We limit the max period to half of the max counter value so
* that even in the case of extreme interrupt latency the
* counter will (hopefully) not wrap past its initial value.
*/
if (hwc->idx == l2_cycle_ctr_idx)
new = BIT_ULL(63);
else
new = BIT_ULL(31);
local64_set(&hwc->prev_count, new);
hml2_pmu__counter_set_value(idx, new);
}
> +static int l2_cache__get_event_idx(struct hml2_pmu *cluster,
> + struct perf_event *event)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + int idx;
> +
> + if (hwc->config_base == L2CYCLE_CTR_RAW_CODE) {
> + if (test_and_set_bit(l2_cycle_ctr_idx, cluster->used_counters))
> + return -EAGAIN;
> +
> + return l2_cycle_ctr_idx;
> + }
> +
> + for (idx = 0; idx < cluster->l2cache_pmu->num_counters - 1; idx++) {
> + if (!test_and_set_bit(idx, cluster->used_counters)) {
> + set_bit(L2_EVT_GROUP(hwc->config_base),
> + cluster->used_groups);
> + return idx;
> + }
> + }
> + /* The counters are all in use. */
> + return -EAGAIN;
> +}
Are we racing with another bitmap manipulation here? Or can we split the find
and set of the bit, i.e.
int num_ctrs = cluster->l2cache_pmu->num_counters;
int idx = find_first_zero_bit(cluster->used_counters, num_ctrs);
if (idx == num_ctrs)
return -EAGAIN;
set_bit(idx, cluster->used_counters)
Otherwise, I think the outer loop can use for_each_clear_bit().
[...]
> +static int l2_cache__event_init(struct perf_event *event)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + struct hml2_pmu *cluster;
> + struct perf_event *sibling;
> + struct l2cache_pmu *l2cache_pmu;
> +
> + if (event->attr.type != event->pmu->type)
> + return -ENOENT;
> +
> + l2cache_pmu = to_l2cache_pmu(event->pmu);
> +
> + if (hwc->sample_period) {
> + dev_warn(&l2cache_pmu->pdev->dev, "Sampling not supported\n");
> + return -EOPNOTSUPP;
> + }
Please remove these dev_warns, or at lease relegate them to ratelimited
debug messages rather than warning messages. Users can easily trigger
these by accident and/or when probing capabilities in the usual fashion,
and there's no need for them.
The CCN driver is an unfortunate bad example in this respect.
[...]
> + /* Don't allow groups with mixed PMUs, except for s/w events */
> + if (event->group_leader->pmu != event->pmu &&
> + !is_software_event(event->group_leader)) {
> + dev_warn(&l2cache_pmu->pdev->dev,
> + "Can't create mixed PMU group\n");
> + return -EINVAL;
> + }
> +
> + list_for_each_entry(sibling, &event->group_leader->sibling_list,
> + group_entry)
> + if (sibling->pmu != event->pmu &&
> + !is_software_event(sibling)) {
> + dev_warn(&l2cache_pmu->pdev->dev,
> + "Can't create mixed PMU group\n");
> + return -EINVAL;
> + }
> +
> + /* Ensure all events in a group are on the same cpu */
> + cluster = get_hml2_pmu(event->cpu);
> + if ((event->group_leader != event) &&
> + (cluster->on_cpu != event->group_leader->cpu)) {
> + dev_warn(&l2cache_pmu->pdev->dev,
> + "Can't create group on CPUs %d and %d",
> + event->cpu, event->group_leader->cpu);
> + return -EINVAL;
> + }
It's probably worth also checking that the events are co-schedulable
(e.g. they don't conflict column-wise).
[...]
> +static void l2_cache__event_start(struct perf_event *event, int flags)
> +{
> + struct hml2_pmu *cluster;
> + struct hw_perf_event *hwc = &event->hw;
> + int idx = hwc->idx;
> + u32 config;
> + u32 event_cc, event_group;
> +
> + hwc->state = 0;
> +
> + cluster = get_hml2_pmu(event->cpu);
> + l2_cache__cluster_set_period(cluster, hwc);
> +
> + if (hwc->config_base == L2CYCLE_CTR_RAW_CODE) {
> + hml2_pmu__set_evccntcr(0x0);
Nit: s/0x0/0/ -- there's no other hex usage to be consistent with, and a
single digit is clearer.
> + } else {
> + config = hwc->config_base;
> + event_cc = L2_EVT_CODE(config);
> + event_group = L2_EVT_GROUP(config);
> +
> + hml2_pmu__set_evcntcr(idx, 0x0);
Nit: s/0x0/0/ -- as above.
> + hml2_pmu__set_evtyper(idx, event_group);
> + hml2_pmu__set_resr(cluster, event_group, event_cc);
> + hml2_pmu__set_evfilter_sys_mode(idx);
> + }
> +
> + hml2_pmu__counter_enable_interrupt(idx);
> + hml2_pmu__counter_enable(idx);
> +}
> +
> +static void l2_cache__event_stop(struct perf_event *event, int flags)
> +{
> + struct hml2_pmu *cluster;
> + struct hw_perf_event *hwc = &event->hw;
> + int idx = hwc->idx;
> +
> + if (!(hwc->state & PERF_HES_STOPPED)) {
It's better to have an early return, e.g.
/* If already stopped, we have nothing to do */
if (hwc->state & PERF_HES_STOPPED)
return;
...
remaining logic here, not in a block
...
> + cluster = get_hml2_pmu(event->cpu);
> + hml2_pmu__counter_disable_interrupt(idx);
> + hml2_pmu__counter_disable(idx);
> +
> + if (flags & PERF_EF_UPDATE)
> + l2_cache__event_update_from_cluster(event, cluster);
> + hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
> + }
> +}
> +
> +static int l2_cache__event_add(struct perf_event *event, int flags)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + int idx;
> + int err = 0;
> + struct hml2_pmu *cluster;
> +
> + cluster = get_hml2_pmu(event->cpu);
> +
> + idx = l2_cache__get_event_idx(cluster, event);
> + if (idx < 0) {
> + err = idx;
> + return err;
> + }
You can return idx directly and avoid the block here.
> + hwc->idx = idx;
> + hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
> + cluster->events[idx] = event;
> + cluster->group_to_counter[L2_EVT_GROUP(hwc->config_base)] = idx;
> + local64_set(&hwc->prev_count, 0ULL);
Nit: s/0ULL/0/
[...]
> +static int l2_cache_filter_match(struct perf_event *event)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + struct hml2_pmu *cluster = get_hml2_pmu(event->cpu);
> + unsigned int group = L2_EVT_GROUP(hwc->config_base);
> +
> + /* check for column exclusion: group already in use by another event */
> + if (test_bit(group, cluster->used_groups) &&
> + cluster->events[cluster->group_to_counter[group]] != event)
> + return 0;
> +
> + return 1;
> +}
I'm still not keen on this. I'm still hoping to kill off filter_match,
as for many reasons it is not great, and I know that x86 have similar
scheduling requirements, and somehow manage without this.
It would be worth adding justification for this in a comment block
above, such as why you need this, and why this won't result in
sub-optimal behaviour.
[...]
> + if (acpi_bus_get_device(ACPI_HANDLE(dev), &device))
> + return -ENODEV;
> +
> + if (kstrtol(device->pnp.unique_id, 10, &fw_cluster_id) < 0) {
> + dev_err(&pdev->dev, "unable to read ACPI uid\n");
> + return -ENODEV;
> + }
> + cluster->l2cache_pmu = l2cache_pmu;
> + for_each_present_cpu(cpu) {
> + if (topology_physical_package_id(cpu) == fw_cluster_id) {
> + cpumask_set_cpu(cpu, &cluster->cluster_cpus);
> + per_cpu(pmu_cluster, cpu) = cluster;
> + }
> + }
I'm still uneasy about this.
The topology_* API doesn't have a well-defined mapping to the MPIDR.Aff*
levels, which itself also don't have a well-defined mapping to your
hardware's clusters (and therefore fw_cluster_id).
Thus, I'm rather worried that this is going to get broken easily, either
by changes in the kernel, or in future HW revisions where the mapping of
clusters to MPIDR.Aff* fields changes.
[...]
> + err = devm_request_irq(&pdev->dev, irq, l2_cache__handle_irq,
> + IRQF_NOBALANCING, "l2-cache-pmu", cluster);
I believe this also needs IRQF_NO_THREAD.
[...]
> + cluster->pmu_lock = __SPIN_LOCK_UNLOCKED(cluster->pmu_lock);
Please use:
spin_lock_init(&cluster->pmu_lock);
[...]
> + platform_set_drvdata(pdev, l2cache_pmu);
> + l2cache_pmu->pmu = (struct pmu) {
> + /* suffix is instance id for future use with multiple sockets */
> + .name = "l2cache_0",
Sorry to go back and forth on this, but as above with the aggregation
comments, we might not need/want the suffix after all, or we might want
per-cluster PMUs.
> + .task_ctx_nr = perf_invalid_context,
> + .pmu_enable = l2_cache__pmu_enable,
> + .pmu_disable = l2_cache__pmu_disable,
> + .event_init = l2_cache__event_init,
> + .add = l2_cache__event_add,
> + .del = l2_cache__event_del,
> + .start = l2_cache__event_start,
> + .stop = l2_cache__event_stop,
> + .read = l2_cache__event_read,
> + .attr_groups = l2_cache_pmu_attr_grps,
> + .filter_match = l2_cache_filter_match,
> + };
As a general note, can we please s/__/_/ for function names? The double
underscore gains us nothing.
[...]
> + l2_counter_present_mask = GENMASK(l2cache_pmu->num_counters - 2, 0) |
> + L2PM_CC_ENABLE;
This looks confusing, and it's the only use of L2PM_CC_ENABLE.
Perhaps use BIT(L2CYCLE_CTR_BIT) instead, and get rid of L2PM_CC_ENABLE?
[...]
> + err = perf_pmu_register(&l2cache_pmu->pmu, l2cache_pmu->pmu.name, -1);
> + if (err < 0) {
> + dev_err(&pdev->dev, "Error %d registering L2 cache PMU\n", err);
> + return err;
> + }
> +
> + dev_info(&pdev->dev, "Registered L2 cache PMU using %d HW PMUs\n",
> + l2cache_pmu->num_pmus);
> +
> + err = cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_QCOM_L2_ONLINE,
> + &l2cache_pmu->node);
> +
> + return err;
> +}
Can we race with hotplug? I believe the hotplug callback registration
should come before the perf_pmu_register()
> +static int l2_cache_pmu_remove(struct platform_device *pdev)
> +{
> + struct l2cache_pmu *l2cache_pmu =
> + to_l2cache_pmu(platform_get_drvdata(pdev));
> +
> + cpuhp_state_remove_instance_nocalls(CPUHP_AP_PERF_ARM_QCOM_L2_ONLINE,
> + &l2cache_pmu->node);
> + perf_pmu_unregister(&l2cache_pmu->pmu);
> + return 0;
> +}
Mirroring the above, I think we should unregister the PMU before
removing the hotplug callbacks.
Thanks,
Mark.
^ permalink raw reply
* [PATCH 1/5] iommu: Allow taking a reference on a group directly
From: Will Deacon @ 2016-11-09 18:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8b35afe8-7e09-c2d3-91ae-5d2a10da6fc8@arm.com>
On Wed, Nov 09, 2016 at 05:46:16PM +0000, Robin Murphy wrote:
> On 09/11/16 17:25, Will Deacon wrote:
> > On Wed, Nov 09, 2016 at 12:47:24PM +0000, Robin Murphy wrote:
> >> iommu_group_get_for_dev() expects that the IOMMU driver's device_group
> >> callback return a group with a reference held for the given device.
> >> Whilst allocating a new group is fine, and pci_device_group() correctly
> >> handles reusing an existing group, there is no general means for IOMMU
> >> drivers doing their own group lookup to take additional references on an
> >> existing group pointer without having to also store device pointers or
> >> resort to elaborate trickery.
> >>
> >> Add an IOMMU-driver-specific function to fill the hole.
> >>
> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> >> ---
> >> drivers/iommu/iommu.c | 14 ++++++++++++++
> >> include/linux/iommu.h | 1 +
> >> 2 files changed, 15 insertions(+)
> >>
> >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> >> index 9a2f1960873b..b0b052bc6bb5 100644
> >> --- a/drivers/iommu/iommu.c
> >> +++ b/drivers/iommu/iommu.c
> >> @@ -552,6 +552,20 @@ struct iommu_group *iommu_group_get(struct device *dev)
> >> EXPORT_SYMBOL_GPL(iommu_group_get);
> >>
> >> /**
> >> + * __iommu_group_get - Increment reference on a group
> >> + * @group: the group to use, must not be NULL
> >> + *
> >> + * This function may be called by internal iommu driver group management
> >> + * when the context of a struct device pointer is not available. It is
> >> + * not for general use. Returns the given group for convenience.
> >> + */
> >> +struct iommu_group *__iommu_group_get(struct iommu_group *group)
> >> +{
> >> + kobject_get(group->devices_kobj);
> >> + return group;
> >> +}
> >
> > This probably either wants sticking in a header or exporting to modules.
> > That said, why do we need the underscores and the comment about internal
> > group management? That's pretty much already the case for iommu_group_get.
>
> The definition of struct iommu_group is private to iommu.c, so any
> touching of the members has to be in here. The comment is to contrast
> with iommu_group_get()'s "This function is called by iommu drivers and
> users". This one is explicitly not for users of the API (DMA mapping,
> VFIO, etc.), as they really have no business messing with refcounts
> directly, and should always be operating in the context of a device;
But they can already do it if they want to, using the horrible group id
hack that's been doing the rounds. The IOMMU API is already low-level
enough, so I don't think trying to split it up like this is helpful.
Hell, people can even just dip in and bump the kobject directly, or grab a
handle to a device in the group already and call iommu_group_get.
That said, it doesn't look like iommu_group_get_by_id actually has any
callers in tree, so maybe we could kill it.
> it's only for the benefit of anyone *implementing* the API. And since
> IOMMU drivers aren't modular (yet... ;)) there's no cause for an export.
>
> > Of course, removing the underscores gives you a naming conflict, but we
> > could just call it something like "iommu_group_get_ref".
>
> Ideally, this would be the iommu_group_get() to precisely match
> iommu_group_put(), and the existing function would renamed something
> like iommu_dev_group_get() (or perhaps even all external uses converted
> over to iommu_group_get_for_dev()), but that would be an awful lot of
> churn for little obvious benefit. Similarly, I nearly added the below
> hunk, but it didn't seem worth the bother.
I'd still rather the new function was renamed. We already have the group,
so calling a weird underscore version of iommu_group_get is really
counter-intuitive.
Joerg -- do you have a preference?
Will
^ permalink raw reply
* [PATCH 1/3] Documentation: DT: Add entry for FSL LS1012A RDB, FRDM, QDS boards
From: Harninder Rai @ 2016-11-09 18:04 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
---
Documentation/devicetree/bindings/arm/fsl.txt | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/fsl.txt b/Documentation/devicetree/bindings/arm/fsl.txt
index d6ee9c6..3b01338 100644
--- a/Documentation/devicetree/bindings/arm/fsl.txt
+++ b/Documentation/devicetree/bindings/arm/fsl.txt
@@ -139,6 +139,22 @@ Example:
Freescale ARMv8 based Layerscape SoC family Device Tree Bindings
----------------------------------------------------------------
+LS1012A SoC
+Required root node properties:
+ - compatible = "fsl,ls1012a";
+
+LS1012A ARMv8 based RDB Board
+Required root node properties:
+ - compatible = "fsl,ls1012a-rdb", "fsl,ls1012a";
+
+LS1012A ARMv8 based FRDM Board
+Required root node properties:
+ - compatible = "fsl,ls1012a-frdm", "fsl,ls1012a";
+
+LS1012A ARMv8 based QDS Board
+Required root node properties:
+ - compatible = "fsl,ls1012a-qds", "fsl,ls1012a";
+
LS1043A SoC
Required root node properties:
- compatible = "fsl,ls1043a";
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3] Documentation: DT: add LS1012A compatible for SCFG and DCFG
From: Harninder Rai @ 2016-11-09 18:04 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
---
Documentation/devicetree/bindings/arm/fsl.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/fsl.txt b/Documentation/devicetree/bindings/arm/fsl.txt
index 3b01338..c9c567a 100644
--- a/Documentation/devicetree/bindings/arm/fsl.txt
+++ b/Documentation/devicetree/bindings/arm/fsl.txt
@@ -108,7 +108,7 @@ status.
- compatible: Should contain a chip-specific compatible string,
Chip-specific strings are of the form "fsl,<chip>-scfg",
The following <chip>s are known to be supported:
- ls1021a, ls1043a, ls1046a, ls2080a.
+ ls1012a, ls1021a, ls1043a, ls1046a, ls2080a.
- reg: should contain base address and length of SCFG memory-mapped registers
@@ -126,7 +126,7 @@ core start address and release the secondary core from holdoff and startup.
- compatible: Should contain a chip-specific compatible string,
Chip-specific strings are of the form "fsl,<chip>-dcfg",
The following <chip>s are known to be supported:
- ls1021a, ls1043a, ls1046a, ls2080a.
+ ls1012a, ls1021a, ls1043a, ls1046a, ls2080a.
- reg : should contain base address and length of DCFG memory-mapped registers
--
2.7.4
^ permalink raw reply related
* [PATCH 3/3] dt-bindings: clockgen: Add compatible string for LS1012A
From: Harninder Rai @ 2016-11-09 18:10 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
---
Documentation/devicetree/bindings/clock/qoriq-clock.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/clock/qoriq-clock.txt b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
index df9cb5a..aa3526f 100644
--- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
+++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
@@ -31,6 +31,7 @@ Required properties:
* "fsl,t4240-clockgen"
* "fsl,b4420-clockgen"
* "fsl,b4860-clockgen"
+ * "fsl,ls1012a-clockgen"
* "fsl,ls1021a-clockgen"
* "fsl,ls1043a-clockgen"
* "fsl,ls1046a-clockgen"
--
2.7.4
^ permalink raw reply related
* [PATCH] arm64: Add DTS support for FSL's LS1012A SoC
From: Harninder Rai @ 2016-11-09 18:11 UTC (permalink / raw)
To: linux-arm-kernel
Add the device tree support for FSL LS1012A SoC.
Following levels of DTSI/DTS files have been created for the LS1012A
SoC family:
- fsl-ls1012a.dtsi:
DTS-Include file for FSL LS1012A SoC.
- fsl-ls1012a-frdm.dts:
DTS file for FSL LS1012A FRDM board.
- fsl-ls1012a-qds.dts:
DTS file for FSL LS1012A QDS board.
- fsl-ls1012a-rdb.dts:
DTS file for FSL LS1012A RDB board.
Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
---
arch/arm64/boot/dts/freescale/Makefile | 3 +
arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts | 115 ++++++++++
arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts | 128 +++++++++++
arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts | 59 +++++
arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 248 +++++++++++++++++++++
5 files changed, 553 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 6602718..39db645 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -1,3 +1,6 @@
+dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-frdm.dtb
+dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-qds.dtb
+dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-rdb.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1043a-qds.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1043a-rdb.dtb
dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1046a-qds.dtb
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
new file mode 100644
index 0000000..1f2da79
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
@@ -0,0 +1,115 @@
+/*
+ * Device Tree file for Freescale LS1012A Freedom Board.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+/dts-v1/;
+
+#include "fsl-ls1012a.dtsi"
+
+/ {
+ model = "LS1012A Freedom Board";
+ compatible = "fsl,ls1012a-frdm", "fsl,ls1012a";
+
+ sys_mclk: clock-mclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <25000000>;
+ };
+
+ reg_1p8v: regulator at 0 {
+ compatible = "regulator-fixed";
+ regulator-name = "1P8V";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+ };
+
+ sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,format = "i2s";
+ simple-audio-card,widgets =
+ "Microphone", "Microphone Jack",
+ "Headphone", "Headphone Jack",
+ "Speaker", "Speaker Ext",
+ "Line", "Line In Jack";
+ simple-audio-card,routing =
+ "MIC_IN", "Microphone Jack",
+ "Microphone Jack", "Mic Bias",
+ "LINE_IN", "Line In Jack",
+ "Headphone Jack", "HP_OUT",
+ "Speaker Ext", "LINE_OUT";
+
+ simple-audio-card,cpu {
+ sound-dai = <&sai2>;
+ frame-master;
+ bitclock-master;
+ };
+
+ simple-audio-card,codec {
+ sound-dai = <&codec>;
+ frame-master;
+ bitclock-master;
+ system-clock-frequency = <25000000>;
+ };
+ };
+};
+
+&i2c0 {
+ status = "okay";
+
+ codec: sgtl5000 at a {
+ #sound-dai-cells = <0>;
+ compatible = "fsl,sgtl5000";
+ reg = <0xa>;
+ VDDA-supply = <®_1p8v>;
+ VDDIO-supply = <®_1p8v>;
+ clocks = <&sys_mclk>;
+ };
+};
+
+&duart0 {
+ status = "okay";
+};
+
+&sai2 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
new file mode 100644
index 0000000..ca680a7
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
@@ -0,0 +1,128 @@
+/*
+ * Device Tree file for Freescale LS1012A QDS Board.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+/dts-v1/;
+
+#include "fsl-ls1012a.dtsi"
+
+/ {
+ model = "LS1012A QDS Board";
+ compatible = "fsl,ls1012a-qds", "fsl,ls1012a";
+
+ sys_mclk: clock-mclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <24576000>;
+ };
+
+ reg_3p3v: regulator at 0 {
+ compatible = "regulator-fixed";
+ regulator-name = "3P3V";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ };
+
+ sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,format = "i2s";
+ simple-audio-card,widgets =
+ "Microphone", "Microphone Jack",
+ "Headphone", "Headphone Jack",
+ "Speaker", "Speaker Ext",
+ "Line", "Line In Jack";
+ simple-audio-card,routing =
+ "MIC_IN", "Microphone Jack",
+ "Microphone Jack", "Mic Bias",
+ "LINE_IN", "Line In Jack",
+ "Headphone Jack", "HP_OUT",
+ "Speaker Ext", "LINE_OUT";
+
+ simple-audio-card,cpu {
+ sound-dai = <&sai2>;
+ frame-master;
+ bitclock-master;
+ };
+
+ simple-audio-card,codec {
+ sound-dai = <&codec>;
+ frame-master;
+ bitclock-master;
+ system-clock-frequency = <24576000>;
+ };
+ };
+};
+
+&i2c0 {
+ status = "okay";
+
+ pca9547 at 77 {
+ compatible = "nxp,pca9547";
+ reg = <0x77>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ i2c at 4 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x4>;
+
+ codec: sgtl5000 at a {
+ #sound-dai-cells = <0>;
+ compatible = "fsl,sgtl5000";
+ reg = <0xa>;
+ VDDA-supply = <®_3p3v>;
+ VDDIO-supply = <®_3p3v>;
+ clocks = <&sys_mclk>;
+ };
+ };
+ };
+};
+
+&duart0 {
+ status = "okay";
+};
+
+&sai2 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
new file mode 100644
index 0000000..924dad6
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
@@ -0,0 +1,59 @@
+/*
+ * Device Tree file for Freescale LS1012A RDB Board.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+/dts-v1/;
+
+#include "fsl-ls1012a.dtsi"
+
+/ {
+ model = "LS1012A RDB Board";
+ compatible = "fsl,ls1012a-rdb", "fsl,ls1012a";
+};
+
+&i2c0 {
+ status = "okay";
+};
+
+&duart0 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
new file mode 100644
index 0000000..0bf5b64
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
@@ -0,0 +1,248 @@
+/*
+ * Device Tree Include file for Freescale Layerscape-1012A family SoC.
+ *
+ * Copyright 2016, Freescale Semiconductor
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPLv2 or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+ compatible = "fsl,ls1012a";
+ interrupt-parent = <&gic>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu0: cpu at 0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x0>;
+ clocks = <&clockgen 1 0>;
+ #cooling-cells = <2>;
+ };
+ };
+
+ sysclk: sysclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <100000000>;
+ clock-output-names = "sysclk";
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ /* Physical Secure PPI */
+ interrupts = <1 13 IRQ_TYPE_LEVEL_LOW>,
+ /* Physical Non-Secure PPI */
+ <1 14 IRQ_TYPE_LEVEL_LOW>,
+ /* Virtual PPI */
+ <1 11 IRQ_TYPE_LEVEL_LOW>,
+ /* Hypervisor PPI */
+ <1 10 IRQ_TYPE_LEVEL_LOW>;
+ };
+
+ pmu {
+ compatible = "arm,armv8-pmuv3";
+ interrupts = <0 106 IRQ_TYPE_LEVEL_LOW>;
+ };
+
+ gic: interrupt-controller at 1400000 {
+ compatible = "arm,gic-400";
+ #interrupt-cells = <3>;
+ interrupt-controller;
+ reg = <0x0 0x1401000 0 0x1000>, /* GICD */
+ <0x0 0x1402000 0 0x2000>, /* GICC */
+ <0x0 0x1404000 0 0x2000>, /* GICH */
+ <0x0 0x1406000 0 0x2000>; /* GICV */
+ interrupts = <1 9 IRQ_TYPE_LEVEL_LOW>;
+ };
+
+ reboot {
+ compatible = "syscon-reboot";
+ regmap = <&dcfg>;
+ offset = <0xb0>;
+ mask = <0x02>;
+ };
+
+ soc {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ clockgen: clocking at 1ee1000 {
+ compatible = "fsl,ls1012a-clockgen";
+ reg = <0x0 0x1ee1000 0x0 0x1000>;
+ #clock-cells = <2>;
+ clocks = <&sysclk>;
+ };
+
+ scfg: scfg at 1570000 {
+ compatible = "fsl,ls1012a-scfg", "syscon";
+ reg = <0x0 0x1570000 0x0 0x10000>;
+ big-endian;
+ };
+
+ dcfg: dcfg at 1ee0000 {
+ compatible = "fsl,ls1012a-dcfg",
+ "syscon";
+ reg = <0x0 0x1ee0000 0x0 0x10000>;
+ big-endian;
+ };
+
+ i2c0: i2c at 2180000 {
+ compatible = "fsl,vf610-i2c";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x0 0x2180000 0x0 0x10000>;
+ interrupts = <0 56 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&clockgen 4 0>;
+ status = "disabled";
+ };
+
+ i2c1: i2c at 2190000 {
+ compatible = "fsl,vf610-i2c";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x0 0x2190000 0x0 0x10000>;
+ interrupts = <0 57 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&clockgen 4 0>;
+ status = "disabled";
+ };
+
+ duart0: serial at 21c0500 {
+ compatible = "fsl,ns16550", "ns16550a";
+ reg = <0x00 0x21c0500 0x0 0x100>;
+ interrupts = <0 54 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clockgen 4 0>;
+ };
+
+ duart1: serial at 21c0600 {
+ compatible = "fsl,ns16550", "ns16550a";
+ reg = <0x00 0x21c0600 0x0 0x100>;
+ interrupts = <0 54 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clockgen 4 0>;
+ };
+
+ gpio0: gpio at 2300000 {
+ compatible = "fsl,qoriq-gpio";
+ reg = <0x0 0x2300000 0x0 0x10000>;
+ interrupts = <0 66 IRQ_TYPE_LEVEL_LOW>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ gpio1: gpio at 2310000 {
+ compatible = "fsl,qoriq-gpio";
+ reg = <0x0 0x2310000 0x0 0x10000>;
+ interrupts = <0 67 IRQ_TYPE_LEVEL_LOW>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ wdog0: wdog at 2ad0000 {
+ compatible = "fsl,ls1012a-wdt",
+ "fsl,imx21-wdt";
+ reg = <0x0 0x2ad0000 0x0 0x10000>;
+ interrupts = <0 83 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&clockgen 4 0>;
+ big-endian;
+ };
+
+ sai1: sai at 2b50000 {
+ #sound-dai-cells = <0>;
+ compatible = "fsl,vf610-sai";
+ reg = <0x0 0x2b50000 0x0 0x10000>;
+ interrupts = <0 148 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&clockgen 4 3>, <&clockgen 4 3>,
+ <&clockgen 4 3>, <&clockgen 4 3>;
+ clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ dma-names = "tx", "rx";
+ dmas = <&edma0 1 47>,
+ <&edma0 1 46>;
+ status = "disabled";
+ };
+
+ sai2: sai at 2b60000 {
+ #sound-dai-cells = <0>;
+ compatible = "fsl,vf610-sai";
+ reg = <0x0 0x2b60000 0x0 0x10000>;
+ interrupts = <0 149 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&clockgen 4 3>, <&clockgen 4 3>,
+ <&clockgen 4 3>, <&clockgen 4 3>;
+ clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ dma-names = "tx", "rx";
+ dmas = <&edma0 1 45>,
+ <&edma0 1 44>;
+ status = "disabled";
+ };
+
+ edma0: edma at 2c00000 {
+ #dma-cells = <2>;
+ compatible = "fsl,vf610-edma";
+ reg = <0x0 0x2c00000 0x0 0x10000>,
+ <0x0 0x2c10000 0x0 0x10000>,
+ <0x0 0x2c20000 0x0 0x10000>;
+ interrupts = <0 103 IRQ_TYPE_LEVEL_LOW>,
+ <0 103 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "edma-tx", "edma-err";
+ dma-channels = <32>;
+ big-endian;
+ clock-names = "dmamux0", "dmamux1";
+ clocks = <&clockgen 4 3>,
+ <&clockgen 4 3>;
+ };
+
+ sata: sata at 3200000 {
+ compatible = "fsl,ls1012a-ahci";
+ reg = <0x0 0x3200000 0x0 0x10000>;
+ interrupts = <0 69 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&clockgen 4 0>;
+ };
+ };
+};
--
2.7.4
^ permalink raw reply related
* [PATCH] arm64: acpi: arch_apei_get_mem_attributes() should use memblock
From: James Morse @ 2016-11-09 18:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu9+T0TYqk+uTUWa5K0E49WXDBpUK4Px_q=52jR0ON1zGA@mail.gmail.com>
Hi Ard,
On 09/11/16 12:12, Ard Biesheuvel wrote:
> On 8 November 2016 at 10:27, James Morse <james.morse@arm.com> wrote:
>> arm64's arch_apei_get_mem_attributes() tries to use
>> efi_mem_attributes() to read the memory attributes from the efi
>> memory map.
>>
>> drivers/firmware/efi/arm-init.c:efi_init() calls efi_memmap_unmap(),
>> which clears the EFI_MEMMAP bit from efi.flags once efi_init() has
>> finished with the memory map. This causes efi_mem_attributes() to
>> return 0 meaning PROT_DEVICE_nGnRnE is the chosen memory type for
>> all ACPI APEI mappings.
>>
>> APEI may call this from NMI context, so we can't re-map the EFI
>> memory map as this may need to allocate virtual address space.
>> 'ghes_ioremap_area' is APEIs cache of virtual address space to
>> access the buffer once we tell it the memory attributes.
>>
>> Do as acpi_os_ioremap() does, and consult memblock to learn if
>> the provided address is memory, or not.
>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>> @@ -241,22 +238,13 @@ void __init acpi_boot_table_init(void)
>> pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>> {
>> /*
>> - * According to "Table 8 Map: EFI memory types to AArch64 memory
>> - * types" of UEFI 2.5 section 2.3.6.1, each EFI memory type is
>> - * mapped to a corresponding MAIR attribute encoding.
>> - * The EFI memory attribute advises all possible capabilities
>> - * of a memory region. We use the most efficient capability.
>> + * The EFI memmap isn't mapped, instead read the version exported
>> + * into memblock. EFI's reserve_regions() call adds memory with the
>> + * WB attribute to memblock via early_init_dt_add_memory_arch().
>> */
>> + if (!memblock_is_memory(addr))
>> + return __pgprot(PROT_DEVICE_nGnRnE);
>>
>> - u64 attr;
>> -
>> - attr = efi_mem_attributes(addr);
>> - if (attr & EFI_MEMORY_WB)
>> - return PAGE_KERNEL;
>> - if (attr & EFI_MEMORY_WT)
>> - return __pgprot(PROT_NORMAL_WT);
>> - if (attr & EFI_MEMORY_WC)
>> - return __pgprot(PROT_NORMAL_NC);
>> - return __pgprot(PROT_DEVICE_nGnRnE);
>> + return PAGE_KERNEL;
>
> As you know, we recently applied [0] to ensure that memory regions
> that are marked by UEFI was write-through cacheable (WT) or
> write-combining (WC) are not misidentified by the kernel as having
> strongly ordered semantics.
Ah, I'd forgotten all about that...
> This means that in this case, you may be returning PAGE_KERNEL for
> regions that are lacking the EFI_MEMORY_WB attribute, which kind of
> defeats the purpose of this function (AIUI), to align the kernel's
> view of the region's attributes with the view of the firmware. I would
> expect that, for use cases like this one, the burden is on the
> firmware to ensure that only a single EFI_MEMORY_xx attribute is set
> if any kind of coherency between UEFI and the kernel is expected. If
> that single attribute is WT or WC, PAGE_KERNEL is most certainly
> wrong.
I've been trying to test some of the APEI code using some hacks on top of
kvmtool. (actually, quite a lot of hacks).
When I trigger an event, I see efi_mem_attributes() always return 0 because the
EFI memory map isn't mapped. This turns out to be because I have 'efi=noruntime'
on the command line. I stopped digging when I found this previously-dead code,
but should have gone further.
If this is an expected interaction, we can ignore this as a bug in my test
setup. (and I have to work out how to get efi runtime services going...)
If not, I can try always mapping the EFI memory map in
arm_enable_runtime_services() if we booted via ACPI, as in this case runtime
services isn't the only user of the memory map.
My intention here was to just mirror acpi_os_ioremap(), which will call
ioremap_cache() for WB/WC/WT regions, which (also) ends up using PROT_NORMAL. We
may get away with this, but you're right, we are less likely to here.
Thanks,
James
^ permalink raw reply
* KASAN & the vmalloc area
From: Dmitry Vyukov @ 2016-11-09 18:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161109105624.GA17020@leverpostej>
On Wed, Nov 9, 2016 at 2:56 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Nov 08, 2016 at 02:09:27PM -0800, Dmitry Vyukov wrote:
>> On Tue, Nov 8, 2016 at 11:03 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>> > When KASAN is selected, we allocate shadow for the whole vmalloc area,
>> > using common zero pte, pmd, pud tables. Walking over these in the ptdump
>> > code takes a *very* long time (I've seen up to 15 minutes with
>> > KASAN_OUTLINE enabled). For DEBUG_WX [3], this means boot hangs for that
>> > long, too.
>
> [...]
>
>> I've seen the same iteration slowness problem on x86 with
>> CONFIG_DEBUG_RODATA which walks all pages. The is about 1 minute, but
>> it is enough to trigger rcu stall warning.
>
> Interesting; do you know where that happens? I can't spot any obvious
> case where we'd have to walk all the page tables for DEBUG_RODATA.
As far as I remember it was this path:
mark_readonly in main.c -> mark_rodata_ro -> debug_checkwx ->
ptdump_walk_pgd_level_checkwx -> ptdump_walk_pgd_level_core.
>> The zero pud and vmalloc-ed stacks looks like different problems.
>> To overcome the slowness we could map zero shadow for vmalloc area lazily.
>> However for vmalloc-ed stacks we need to map actual memory, because
>> stack instrumentation will read/write into the shadow.
>
> Sure. The point I was trying to make is that there' be fewer page tables
> to walk (unless the vmalloc area was exhausted), assuming we also lazily
> mapped the common zero shadow for the vmalloc area.
>
>> One downside here is that vmalloc shadow can be as large as 1:1 (if we
>> allocate 1 page in vmalloc area we need to allocate 1 page for
>> shadow).
>
> I thought per prior discussion we'd only need to allocate new pages for
> the stacks in the vmalloc region, and we could re-use the zero pages?
We can't reuse zero ro pages for stacks, because stack instrumentation
writes to stack shadow.
When we have a large continuous range of memory, shadow for it is
1/8th. However, if we have a separate page, we will need to map whole
page of shadow for it, i.e. 1:1 shadow overhead.
> ... or are you trying to quantify the cost of the page tables?
>
>> Re slowness: could we just skip the KASAN zero puds (the top level)
>> while walking? Can they be interesting for anybody?
>
> They're interesting for the ptdump case (which allows privileged users
> to dump the tables via /sys/kernel/debug/kernel_page_tables). I've seen
> 25+ minute hangs there.
>
>> We can just pretend that they are not there. Looks like a trivial
>> solution for the problem at hand.
>
> For the boot time hang it's option. Though I'd prefer that the sanity
> checks applied to all of tables, shadow regions included.
>
> Thanks,
> Mark.
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe at googlegroups.com.
> To post to this group, send email to kasan-dev at googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20161109105624.GA17020%40leverpostej.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [PATCH v7] soc: qcom: add l2 cache perf events driver
From: Will Deacon @ 2016-11-09 18:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161109175413.GE17020@leverpostej>
On Wed, Nov 09, 2016 at 05:54:13PM +0000, Mark Rutland wrote:
> On Fri, Oct 28, 2016 at 04:50:13PM -0400, Neil Leeder wrote:
> > + struct perf_event *events[MAX_L2_CTRS];
> > + struct l2cache_pmu *l2cache_pmu;
> > + DECLARE_BITMAP(used_counters, MAX_L2_CTRS);
> > + DECLARE_BITMAP(used_groups, L2_EVT_GROUP_MAX + 1);
> > + int group_to_counter[L2_EVT_GROUP_MAX + 1];
> > + int irq;
> > + /* The CPU that is used for collecting events on this cluster */
> > + int on_cpu;
> > + /* All the CPUs associated with this cluster */
> > + cpumask_t cluster_cpus;
>
> I'm still uncertain about aggregating all cluster PMUs into a larger
> PMU, given the cluster PMUs are logically independent (at least in terms
> of the programming model).
>
> However, from what I understand the x86 uncore PMU drivers aggregate
> symmetric instances of uncore PMUs (and also aggregate across packages
> to the same logical PMU).
>
> Whatever we do, it would be nice for the uncore drivers to align on a
> common behaviour (and I think we're currently going the oppposite route
> with Cavium's uncore PMU). Will, thoughts?
I'm not a big fan of aggregating this stuff. Ultimately, the user in the
driving seat of perf is going to need some knowledge about the toplogy of
the system in order to perform sensible profiling using an uncore PMU.
If the kernel tries to present a single, unified PMU then we paint ourselves
into a corner when the hardware isn't as symmetric as we want it to be
(big/little on the CPU side is the extreme example of this). If we want
to be consistent, then exposing each uncore unit as a separate PMU is
the way to go. That doesn't mean we can't aggregate the components of a
distributed PMU (e.g. the CCN or the SMMU), but we don't want to aggregate
at the programming interface/IP block level.
We could consider exposing some topology information in sysfs if that's
seen as an issue with the non-aggregated case.
Will
^ permalink raw reply
* KASAN & the vmalloc area
From: Dmitry Vyukov @ 2016-11-09 18:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8abab822-4d93-a336-b00f-7aa9b1f47f8c@virtuozzo.com>
On Wed, Nov 9, 2016 at 8:53 AM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
> On 11/08/2016 10:03 PM, Mark Rutland wrote:
>> Hi,
>>
>> I see a while back [1] there was a discussion of what to do about KASAN
>> and vmapped stacks, but it doesn't look like that was solved, judging by
>> the vmapped stacks pull [2] for v4.9.
>>
>> I wondered whether anyone had looked at that since?
>>
>
> Sadly, but I didn't have much time for this yet, so it's in an initial state.
>
>> I have an additional reason to want to dynamically allocate the vmalloc
>> area shadow: it turns out that KASAN currently interacts rather poorly
>> with the arm64 ptdump code.
>>
>> When KASAN is selected, we allocate shadow for the whole vmalloc area,
>> using common zero pte, pmd, pud tables. Walking over these in the ptdump
>> code takes a *very* long time (I've seen up to 15 minutes with
>> KASAN_OUTLINE enabled). For DEBUG_WX [3], this means boot hangs for that
>> long, too.
>>
>
> I didn't look at any code, but we probably could can remember last visited pgd
> and skip next pgd if it's the same as previous.
Good idea.
> Alternatively - just skip kasan_zero_p*d in ptdump walker.
Mark have concern with the fact that we hide the mapping from the
walker this way. But your above idea with deduping pgd's during walk
both fast and doesn't hide anything from walker.
>> If I don't allocate vmalloc shadow (and remove the apparently pointlesss
>> shadow of the shadow area), and only allocate shadow for the image,
>> fixmap, vmemmap and so on, that delay gets cut to a few seconds, which
>> is tolerable for a debug configuration...
>>
>> ... however, things blow up when the kernel touches vmalloc'd memory for
>> the first time, as we don't install shadow for that dynamically.
>>
>> Thanks,
>> Mark.
>>
>> [1] https://lkml.kernel.org/r/CALCETrWucrYp+yq8RHSDqf93xtg793duByirurzJbLRhrz=tcA at mail.gmail.com
>> [2] https://lkml.kernel.org/r/20161003092940.GA691 at gmail.com
>> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/464191.html
>>
^ permalink raw reply
* [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Rob Herring @ 2016-11-09 18:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ee296deafdcbeb431a592b591ae38a758ba4cce7.1477911954.git-series.gregory.clement@free-electrons.com>
On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:
> From: Ziji Hu <huziji@marvell.com>
>
> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> Add Xenon-specific properties.
> Also add properties for Xenon PHY setting.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
> MAINTAINERS | 1 +-
> 2 files changed, 162 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> new file mode 100644
> index 000000000000..0d2d139494d3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> @@ -0,0 +1,161 @@
> +Marvell's Xenon SDHCI Controller device tree bindings
> +This file documents differences between the core mmc properties
> +described by mmc.txt and the properties used by the Xenon implementation.
> +
> +A single Xenon IP can support multiple slots.
> +Each slot acts as an independent SDHC. It owns independent resources, such
> +as register sets clock and PHY.
> +Each slot should have an independent device tree node.
> +
> +Required Properties:
> +- compatible: should be one of the following
> + - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
> + Must provide a second register area and marvell,pad-type.
> + - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
> + Armada-3700.
Need SoC specific compatible strings.
> +
> +- clocks:
> + Array of clocks required for SDHCI.
> + Requires at least one for Xenon IP core.
> + Some SOCs require additional clock for AXI bus.
> +
> +- clock-names:
> + Array of names corresponding to clocks property.
> + The input clock for Xenon IP core should be named as "core".
> + The optional AXI clock should be named as "axi".
When is AXI clock optional? This should be required for ?? compatible
strings.
> +
> +- reg:
> + * For "marvell,xenon-sdhci", one register area for Xenon IP.
> +
> + * For "marvell,armada-3700-sdhci", two register areas.
> + The first one for Xenon IP register. The second one for the Armada 3700 SOC
> + PHY PAD Voltage Control register.
> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
> + in below.
> + Please also check property marvell,pad-type in below.
> +
> +Optional Properties:
> +- marvell,xenon-slotno:
Multiple slots should be represented as child nodes IMO. I think some
other bindings already do this.
> + Indicate the corresponding bit index of current Xenon SDHC slot in
> + SDHC System Operation Control Register Bit[7:0].
> + Set/clear the corresponding bit to enable/disable current Xenon SDHC
> + slot.
> + If this property is not provided, Xenon IP should contain only one
> + slot.
> +
> +- marvell,xenon-phy-type:
> + Xenon support mutilple types of PHYs.
> + To select eMMC 5.1 PHY, set:
> + marvell,xenon-phy-type = "emmc 5.1 phy"
> + eMMC 5.1 PHY is the default choice if this property is not provided.
> + To select eMMC 5.0 PHY, set:
> + marvell,xenon-phy-type = "emmc 5.0 phy"
> + To select SDH PHY, set:
> + marvell,xenon-phy-type = "sdh phy"
> + Please note that eMMC PHY is a general PHY for eMMC/SD/SDIO, other than for
> + eMMC only.
Does this vary per instance on a single SoC? If not, then an SoC
specific compatible should determine this.
Also, the " phy" part is redundant.
> +
> +- marvell,xenon-phy-znr:
> + Set PHY ZNR value.
> + Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
> + valid range = [0:0x1F].
> + ZNR is set as 0xF by default if this property is not provided.
> +
> +- marvell,xenon-phy-zpr:
> + Set PHY ZPR value.
> + Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
> + valid range = [0:0x1F].
> + ZPR is set as 0xF by default if this property is not provided.
> +
> +- marvell,xenon-phy-nr-success-tun:
> + Set the number of required consecutive successful sampling points used to
> + identify a valid sampling window, in tuning process.
> + Valid range = [1:7]. Set as 0x4 by default if this property is not provided.
> +
> +- marvell,xenon-phy-tun-step-divider:
> + Set the divider for calculating TUN_STEP.
> + Set as 64 by default if this property is not provided.
> +
> +- marvell,xenon-phy-slow-mode:
> + Force PHY into slow mode.
> + Only available when bus frequency lower than 50MHz in SDR mde.
> + Disabled by default. Please do not enable it unless it is necessary.
> +
> +- marvell,xenon-mask-conflict-err:
> + Mask Conflict Error alert on some SOC. Disabled by default.
> +
> +- marvell,xenon-tun-count:
> + Xenon SDHC SOC usually doesn't provide re-tuning counter in
> + Capabilities Register 3 Bit[11:8].
> + This property provides the re-tuning counter.
> + If this property is not set, default re-tuning counter will
> + be set as 0x9 in driver.
> +
> +- marvell,pad-type:
> + Type of Armada 3700 SOC PHY PAD Voltiage Controller register.
> + Only valid when "marvell,armada-3700-sdhci" is selected.
> + Two types: "sd" and "fixed-1-8v".
> + If "sd" is slected, SOC PHY PAD is set as 3.3V at the beginning and is
> + switched to 1.8V when SD in UHS-I.
> + If "fixed-1-8v" is slected, SOC PHY PAD is fixed 1.8V, such as for eMMC.
> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
> + in below.
> +
> +Example:
> +- For eMMC slot:
> +
> + sdhci at aa0000 {
> + compatible = "marvell,xenon-sdhci";
> + reg = <0xaa0000 0x1000>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
> + clocks = <&emmc_clk>, <&axi_clock>;
> + clock-names = "core", "axi";
> + bus-width = <8>;
> + marvell,xenon-emmc;
Not documented. If we need to specify the type of slot/card, then we
need to come up with a standard property. This was either already done
or attempted IIRC.
> + marvell,xenon-slotno = <0>;
> + marvell,xenon-phy-type = "emmc 5.1 phy";
> + marvell,xenon-tun-count = <11>;
> + };
> +
> +- For SD/SDIO slot:
> +
> + sdhci at ab0000 {
> + compatible = "marvell,xenon-sdhci";
> + reg = <0xab0000 0x1000>;
> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
> + vqmmc-supply = <&sd_regulator>;
> + clocks = <&sdclk>;
> + clock-names = "core";
> + bus-width = <4>;
> + marvell,xenon-tun-count = <9>;
> + };
> +
> +- For eMMC slot with compatible "marvell,armada-3700-sdhci":
> +
> + sdhci at aa0000 {
> + compatible = "marvell,armada-3700-sdhci";
> + reg = <0xaa0000 0x1000>,
> + <phy_addr 0x4>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
> + clocks = <&emmcclk>;
> + clock-names = "core";
> + bus-width = <8>;
> + marvell,xenon-emmc;
> +
> + marvell,pad-type = "fixed-1-8v";
> + };
> +
> +- For SD/SDIO slot with compatible "marvell,armada-3700-sdhci":
> +
> + sdhci at ab0000 {
> + compatible = "marvell,armada-3700-sdhci";
> + reg = <0xab0000 0x1000>,
> + <phy_addr 0x4>;
> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
> + vqmmc-supply = <&sd_regulator>;
> + clocks = <&sdclk>;
> + clock-names = "core";
> + bus-width = <4>;
> +
> + marvell,pad-type = "sd";
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1a5c4c30ea24..850a0afb0c8d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7608,6 +7608,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
> M: Ziji Hu <huziji@marvell.com>
> L: linux-mmc at vger.kernel.org
> S: Supported
> +F: Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>
> MATROX FRAMEBUFFER DRIVER
> L: linux-fbdev at vger.kernel.org
> --
> git-series 0.8.10
^ permalink raw reply
* [PATCH v2 5/7] ARM: shmobile: Document DT bindings for CCCR and PRR
From: Rob Herring @ 2016-11-09 18:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477913455-5314-6-git-send-email-geert+renesas@glider.be>
On Mon, Oct 31, 2016 at 12:30:53PM +0100, Geert Uytterhoeven wrote:
> Add device tree binding documentation for the Common Chip Code Register
> and Product Register, which provide SoC product and revision
> information.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
> - New.
> ---
> Documentation/devicetree/bindings/arm/shmobile.txt | 26 ++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Rob Herring @ 2016-11-09 18:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477925138-23457-2-git-send-email-bgolaszewski@baylibre.com>
On Mon, Oct 31, 2016 at 03:45:34PM +0100, Bartosz Golaszewski wrote:
> Create a new driver for the da8xx DDR2/mDDR controller and implement
> support for writing to the Peripheral Bus Burst Priority Register.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> .../memory-controllers/ti-da8xx-ddrctl.txt | 20 +++
Acked-by: Rob Herring <robh@kernel.org>
> drivers/memory/Kconfig | 8 +
> drivers/memory/Makefile | 1 +
> drivers/memory/da8xx-ddrctl.c | 175 +++++++++++++++++++++
> 4 files changed, 204 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
> create mode 100644 drivers/memory/da8xx-ddrctl.c
^ permalink raw reply
* [PATCH v2 2/5] ARM: bus: da8xx-mstpri: new driver
From: Rob Herring @ 2016-11-09 18:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477925138-23457-3-git-send-email-bgolaszewski@baylibre.com>
On Mon, Oct 31, 2016 at 03:45:35PM +0100, Bartosz Golaszewski wrote:
> Create the driver for the da8xx master peripheral priority
> configuration and implement support for writing to the three
> Master Priority registers on da850 SoCs.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> .../devicetree/bindings/bus/ti,da850-mstpri.txt | 20 ++
Acked-by: Rob Herring <robh@kernel.org>
> drivers/bus/Kconfig | 9 +
> drivers/bus/Makefile | 2 +
> drivers/bus/da8xx-mstpri.c | 269 +++++++++++++++++++++
> 4 files changed, 300 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
> create mode 100644 drivers/bus/da8xx-mstpri.c
^ permalink raw reply
* [PATCH] arm64: acpi: arch_apei_get_mem_attributes() should use memblock
From: Will Deacon @ 2016-11-09 18:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5823677D.3050803@arm.com>
On Wed, Nov 09, 2016 at 06:14:21PM +0000, James Morse wrote:
> On 09/11/16 12:12, Ard Biesheuvel wrote:
> > This means that in this case, you may be returning PAGE_KERNEL for
> > regions that are lacking the EFI_MEMORY_WB attribute, which kind of
> > defeats the purpose of this function (AIUI), to align the kernel's
> > view of the region's attributes with the view of the firmware. I would
> > expect that, for use cases like this one, the burden is on the
> > firmware to ensure that only a single EFI_MEMORY_xx attribute is set
> > if any kind of coherency between UEFI and the kernel is expected. If
> > that single attribute is WT or WC, PAGE_KERNEL is most certainly
> > wrong.
>
> I've been trying to test some of the APEI code using some hacks on top of
> kvmtool. (actually, quite a lot of hacks).
>
> When I trigger an event, I see efi_mem_attributes() always return 0 because the
> EFI memory map isn't mapped. This turns out to be because I have 'efi=noruntime'
> on the command line. I stopped digging when I found this previously-dead code,
> but should have gone further.
>
> If this is an expected interaction, we can ignore this as a bug in my test
> setup. (and I have to work out how to get efi runtime services going...)
>
> If not, I can try always mapping the EFI memory map in
> arm_enable_runtime_services() if we booted via ACPI, as in this case runtime
> services isn't the only user of the memory map.
>
>
> My intention here was to just mirror acpi_os_ioremap(), which will call
> ioremap_cache() for WB/WC/WT regions, which (also) ends up using PROT_NORMAL. We
> may get away with this, but you're right, we are less likely to here.
I'd certainly be interested in opinions as to what acpi_os_ioremap is
supposed to do, since it has fingers in the NOMAP pie and if we change the
polarity of pfn_valid() for NOMAP mappings (as suggested by Robert Richter
to fix his NUMA issue), then acpi_os_ioremap will actually *fail* for
these WB/WC regions.
Will
^ permalink raw reply
* [RFC PATCH v2 1/5] net: mdio-mux-mmioreg: Add support for 16bit and 32bit register sizes
From: Rob Herring @ 2016-11-09 18:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477932987-27871-2-git-send-email-narmstrong@baylibre.com>
On Mon, Oct 31, 2016 at 05:56:23PM +0100, Neil Armstrong wrote:
> In order to support PHY switching on Amlogic GXL SoCs, add support for
> 16bit and 32bit registers sizes.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> .../devicetree/bindings/net/mdio-mux-mmioreg.txt | 4 +-
> drivers/net/phy/mdio-mux-mmioreg.c | 60 +++++++++++++++++-----
> 2 files changed, 49 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt b/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
> index 8516929..065e8bd 100644
> --- a/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
> +++ b/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
> @@ -3,7 +3,7 @@ Properties for an MDIO bus multiplexer controlled by a memory-mapped device
> This is a special case of a MDIO bus multiplexer. A memory-mapped device,
> like an FPGA, is used to control which child bus is connected. The mdio-mux
> node must be a child of the memory-mapped device. The driver currently only
As you're touching this sentence, this describes the binding, not a
driver. With that,
Acked-by: Rob Herring <robh@kernel.org>
> -supports devices with eight-bit registers.
> +supports devices with 8, 16 or 32-bit registers.
>
> Required properties in addition to the generic multiplexer properties:
>
> @@ -11,7 +11,7 @@ Required properties in addition to the generic multiplexer properties:
>
> - reg : integer, contains the offset of the register that controls the bus
> multiplexer. The size field in the 'reg' property is the size of
> - register, and must therefore be 1.
> + register, and must therefore be 1, 2, or 4.
>
> - mux-mask : integer, contains an eight-bit mask that specifies which
> bits in the register control the actual bus multiplexer. The
^ permalink raw reply
* [PATCH v2 1/3] binding: irqchip: mtk-cirq: Add binding document
From: Rob Herring @ 2016-11-09 18:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478001122-8664-2-git-send-email-youlin.pei@mediatek.com>
On Tue, Nov 01, 2016 at 07:52:00PM +0800, Youlin Pei wrote:
> This commit adds the device tree binding document for
> the mediatek cirq.
>
> Signed-off-by: Youlin Pei <youlin.pei@mediatek.com>
> ---
> .../interrupt-controller/mediatek,cirq.txt | 30 ++++++++++++++++++++
> 1 file changed, 30 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt b/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> new file mode 100644
> index 0000000..84e8123
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> @@ -0,0 +1,30 @@
> +* Mediatek 27xx cirq
> +
> +In Mediatek SOCs, the CIRQ is a low power interrupt controller designed to
> +works outside MCUSYS which comprises with Cortex-Ax cores,CCI and GIC.
s/works/work/
> +The external interrupts (outside MCUSYS) will feed through CIRQ and connect
> +to GIC in MCUSYS. When CIRQ is enabled, it will record the edge-sensitive
> +interrupts and generated a pulse signal to parent interrupt controller when
s/generated/generate/
> +flush command is executed. With CIRQ, MCUSYS can be completely turned off
> +to improve the system power consumption without losing interrupts.
> +
> +Required properties:
> +- compatible: should be: "mediatek,mtk-cirq".
This should be SoC specific. This is fine as a fallback if the same
block is in many SoCs, but mediatek and mtk is a bit redundant.
> +- interrupt-controller : Identifies the node as an interrupt controller.
> +- #interrupt-cells : Use the same format as specified by GIC in arm,gic.txt.
> +- interrupt-parent: phandle of irq parent for cirq. The parent must
> + use the same interrupt-cells format as GIC.
> +- reg: Physical base address of the cirq registers and length of memory
> + mapped region.
> +- mediatek,ext-irq-start: Identifies external irq start number in different
> + SOCs.
Wouldn't this always be 32 if the GIC is the parent? If 32 is the common
case, then use the SoC compatible to determine this value.
> +
> +Example:
> + cirq: interrupt-controller at 10204000 {
> + compatible = "mediatek,mtk-cirq";
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + interrupt-parent = <&sysirq>;
> + reg = <0 0x10204000 0 0x4000>;
> + mediatek,ext-irq-start = <32>;
> + };
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCH v2 1/2] mmc: sdhci-iproc: Add brcm,sdhci-iproc compat string in bindings document
From: Rob Herring @ 2016-11-09 18:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478018277-10097-2-git-send-email-scott.branden@broadcom.com>
On Tue, Nov 01, 2016 at 09:37:56AM -0700, Scott Branden wrote:
> Adds brcm,sdhci-iproc compat string to DT bindings document for
> the iProc SDHCI driver.
>
> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
> ---
> Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt | 9 +++++++++
> 1 file changed, 9 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v2 4/9] regulator: lp873x: Add support for populating input supply
From: Rob Herring @ 2016-11-09 18:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e5619d78-5752-c8fb-8a11-6629c4b7c3f6@ti.com>
On Wed, Nov 02, 2016 at 10:58:40AM +0530, Lokesh Vutla wrote:
>
>
> On Monday 31 October 2016 02:11 AM, Rob Herring wrote:
> > On Fri, Oct 21, 2016 at 04:08:36PM +0530, Lokesh Vutla wrote:
> >> In order to have a proper topology of regulators for a platform, each
> >> registering regulator needs to populate supply_name field for identifying
> >> its supply's name. Add supply_name field for lp873x regulators.
> >>
> >> Cc: Lee Jones <lee.jones@linaro.org>
> >> Cc: Keerthy <j-keerthy@ti.com>
> >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> >> ---
> >> Documentation/devicetree/bindings/mfd/lp873x.txt | 8 ++++++++
> >> drivers/regulator/lp873x-regulator.c | 1 +
> >> 2 files changed, 9 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/lp873x.txt b/Documentation/devicetree/bindings/mfd/lp873x.txt
> >> index 52766c2..998837a 100644
> >> --- a/Documentation/devicetree/bindings/mfd/lp873x.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/lp873x.txt
> >> @@ -7,6 +7,9 @@ Required properties:
> >> - #gpio-cells: Should be two. The first cell is the pin number and
> >> the second cell is used to specify flags.
> >> See ../gpio/gpio.txt for more information.
> >> + - xxx-in-supply: Phandle to parent supply node of each regulator
> >> + populated under regulators node. xxx should match
> >> + the supply_name populated in driver.
> >
> > The driver is irrelevant. This should reference a list in this document.
>
> okay. See if the below updated patch is fine.
>
> -----------------------------8<----------------------------8<----------------------------
> From 666f925423fa35c7bfcc77fa3c883cbea5d8ef8e Mon Sep 17 00:00:00 2001
> From: Lokesh Vutla <lokeshvutla@ti.com>
> Date: Wed, 21 Sep 2016 11:50:49 +0530
> Subject: [PATCH v3] regulator: lp873x: Add support for populating input
> supply
>
> In order to have a proper topology of regulators for a platform, each
> registering regulator needs to populate supply_name field for identifying
> its supply's name. Add supply_name field for lp873x regulators.
>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> Documentation/devicetree/bindings/mfd/lp873x.txt | 8 ++++++++
> drivers/regulator/lp873x-regulator.c | 1 +
> 2 files changed, 9 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH 1/3] ipmi/bt-bmc: change compatible node to 'aspeed, ast2400-ibt-bmc'
From: Rob Herring @ 2016-11-09 18:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478073426-3714-2-git-send-email-clg@kaod.org>
On Wed, Nov 02, 2016 at 08:57:04AM +0100, C?dric Le Goater wrote:
> The Aspeed SoCs have two BT interfaces : one is IPMI compliant and the
> other is H8S/2168 compliant.
>
> The current ipmi/bt-bmc driver implements the IPMI version and we
> should reflect its nature in the compatible node name using
> 'aspeed,ast2400-ibt-bmc' instead of 'aspeed,ast2400-bt-bmc'. The
> latter should be used for a H8S interface driver if it is implemented
> one day.
>
> Signed-off-by: C?dric Le Goater <clg@kaod.org>
> ---
> .../ipmi/{aspeed,ast2400-bt-bmc.txt => aspeed,ast2400-ibt-bmc.txt} | 4 ++--
Acked-by: Rob Herring <robh@kernel.org>
> drivers/char/ipmi/bt-bmc.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
> rename Documentation/devicetree/bindings/ipmi/{aspeed,ast2400-bt-bmc.txt => aspeed,ast2400-ibt-bmc.txt} (85%)
^ permalink raw reply
* [PATCH v3 2/2] dt-bindings: net: Add OXNAS DWMAC Bindings
From: Rob Herring @ 2016-11-09 18:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161102140237.6955-3-narmstrong@baylibre.com>
On Wed, Nov 02, 2016 at 03:02:37PM +0100, Neil Armstrong wrote:
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> .../devicetree/bindings/net/oxnas-dwmac.txt | 39 ++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/oxnas-dwmac.txt
Acked-by: Rob Herring <robh@kernel.org>
^ 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