* [PATCH 2/2] perf: add arm64 smmuv3 pmu driver
From: Linu Cherian @ 2017-12-05 5:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1de250db-87d1-ed85-d0a7-43319139412b@arm.com>
Hi Robin,
On Mon Aug 07, 2017 at 03:31:24PM +0100, Robin Murphy wrote:
> On 04/08/17 20:59, Neil Leeder wrote:
> > Adds a new driver to support the SMMU v3 PMU and add it into the
> > perf events framework.
> >
> > Each SMMU node may have multiple PMUs associated with it, each of
> > which may support different events.
> >
> > PMUs are named smmu_0_<phys_addr_page> where <phys_addr_page>
> > is the physical page address of the SMMU PMCG.
> > For example, the SMMU PMCG at 0xff88840000 is named smmu_0_ff88840
>
> This seems a bit rough - is it at feasible to at least chase the node
> reference and namespace them by the associated component, e.g. something
> like "arm-smmu-v3.x:pmcg.y"? The user can always dig the associated
> physical address out of /proc/iomem if necessary.
>
Assuming x indicates the smmu v3 node id and y indicates pmcg group id.
If are making such a assumption,
# Would like to clarified if the architecture gurantees that a smmu pmcg group is always
associated with a specific SMMU node.
From, Section 10.2 in SMMU Architecture Specification
"A counter group is conceptually free-standing and has no interdependency on the behavior of other counter
groups or even on the SMMU configuration itself"
Does the above and also Fig 19 from the same section,
imply that a PMCG group need not be restricted to a SMMU node ?
> > Filtering by stream id is done by specifying filtering parameters
> > with the event. Options are:
> > filter_enable - 0 = no filtering, 1 = filtering enabled
> > filter_span - 0 = exact match, 1 = pattern match
> > filter_sec - applies to non-secure (0) or secure (1) namespace
>
> I'm a little dubious as to how useful it is to expose this, since we
> can't see the true value of SCR.SO so have no way of knowing what we'll
> actually end up counting.
>
> > filter_stream_id - pattern to filter against
> > Further filtering information is available in the SMMU documentation.
> >
> > Example: perf stat -e smmu_0_ff88840/transaction,filter_enable=1,
> > filter_span=1,filter_stream_id=0x42/ -a pwd
> > Applies filter pattern 0x42 to transaction events.
> >
> > SMMU events are not attributable to a CPU, so task mode and sampling
> > are not supported.
> >
> > Signed-off-by: Neil Leeder <nleeder@codeaurora.org>
> > ---
> > drivers/perf/Kconfig | 9 +
> > drivers/perf/Makefile | 1 +
> > drivers/perf/arm_smmuv3_pmu.c | 813 ++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 823 insertions(+)
> > create mode 100644 drivers/perf/arm_smmuv3_pmu.c
> >
> > diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> > index e5197ff..e7721d1 100644
> > --- a/drivers/perf/Kconfig
> > +++ b/drivers/perf/Kconfig
> > @@ -17,6 +17,15 @@ config ARM_PMU_ACPI
> > depends on ARM_PMU && ACPI
> > def_bool y
> >
> > +config ARM_SMMUV3_PMU
> > + bool "ARM SMMUv3 PMU"
> > + depends on PERF_EVENTS && ARM64 && ACPI
>
> PERF_EVENTS is already a top-level dependency now.
>
> > + help
> > + Provides support for the SMMU version 3 performance monitor unit (PMU)
> > + on ARM-based systems.
> > + Adds the SMMU PMU into the perf events subsystem for
> > + monitoring SMMU performance events.
> > +
> > config QCOM_L2_PMU
> > bool "Qualcomm Technologies L2-cache PMU"
> > depends on ARCH_QCOM && ARM64 && ACPI
> > diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> > index 6420bd4..3012f5e 100644
> > --- a/drivers/perf/Makefile
> > +++ b/drivers/perf/Makefile
> > @@ -1,5 +1,6 @@
> > obj-$(CONFIG_ARM_PMU) += arm_pmu.o arm_pmu_platform.o
> > obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
> > +obj-$(CONFIG_ARM_SMMUV3_PMU) += arm_smmuv3_pmu.o
> > obj-$(CONFIG_QCOM_L2_PMU) += qcom_l2_pmu.o
> > obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
> > obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
> > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
> > new file mode 100644
> > index 0000000..1e70791
> > --- /dev/null
> > +++ b/drivers/perf/arm_smmuv3_pmu.c
> > @@ -0,0 +1,813 @@
> > +/* Copyright (c) 2017 The Linux Foundation. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * This program 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.
> > + */
> > +
> > +/*
> > + * This driver adds support for perf events to use the Performance
> > + * Monitor Counter Groups (PMCG) associated with an SMMUv3 node
> > + * to monitor that node.
> > + *
> > + * Devices are named smmu_0_<phys_addr_page> where <phys_addr_page>
> > + * is the physical page address of the SMMU PMCG.
> > + * For example, the SMMU PMCG at 0xff88840000 is named smmu_0_ff88840
> > + *
> > + * Filtering by stream id is done by specifying filtering parameters
> > + * with the event. options are:
> > + * filter_enable - 0 = no filtering, 1 = filtering enabled
> > + * filter_span - 0 = exact match, 1 = pattern match
> > + * filter_sec - filter applies to non-secure (0) or secure (1) namespace
> > + * filter_stream_id - pattern to filter against
> > + * Further filtering information is available in the SMMU documentation.
> > + *
> > + * Example: perf stat -e smmu_0_ff88840/transaction,filter_enable=1,
> > + * filter_span=1,filter_stream_id=0x42/ -a pwd
> > + * Applies filter pattern 0x42 to transaction events.
> > + *
> > + * SMMU events are not attributable to a CPU, so task mode and sampling
> > + * are not supported.
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/acpi_iort.h>
> > +#include <linux/bitops.h>
> > +#include <linux/cpuhotplug.h>
> > +#include <linux/cpumask.h>
> > +#include <linux/device.h>
> > +#include <linux/errno.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/irq.h>
> > +#include <linux/kernel.h>
> > +#include <linux/list.h>
> > +#include <linux/msi.h>
>
> Is MSI support planned?
>
> > +#include <linux/perf_event.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/smp.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/types.h>
> > +
> > +#include <asm/local64.h>
> > +
> > +#define SMMU_PMCG_EVCNTR0 0x0
> > +#define SMMU_PMCG_EVCNTR(n, stride) (SMMU_PMCG_EVCNTR0 + (n) * (stride))
> > +#define SMMU_PMCG_EVTYPER0 0x400
> > +#define SMMU_PMCG_EVTYPER(n) (SMMU_PMCG_EVTYPER0 + (n) * 4)
> > +#define SMMU_PMCG_EVTYPER_SEC_SID_SHIFT 30
> > +#define SMMU_PMCG_EVTYPER_SID_SPAN_SHIFT 29
> > +#define SMMU_PMCG_EVTYPER_EVENT_MASK GENMASK(15, 0)
> > +#define SMMU_PMCG_SVR0 0x600
> > +#define SMMU_PMCG_SVR(n, stride) (SMMU_PMCG_SVR0 + (n) * (stride))
> > +#define SMMU_PMCG_SMR0 0xA00
> > +#define SMMU_PMCG_SMR(n) (SMMU_PMCG_SMR0 + (n) * 4)
> > +#define SMMU_PMCG_CNTENSET0 0xC00
> > +#define SMMU_PMCG_CNTENCLR0 0xC20
> > +#define SMMU_PMCG_INTENSET0 0xC40
> > +#define SMMU_PMCG_INTENCLR0 0xC60
> > +#define SMMU_PMCG_OVSCLR0 0xC80
> > +#define SMMU_PMCG_OVSSET0 0xCC0
> > +#define SMMU_PMCG_CAPR 0xD88
> > +#define SMMU_PMCG_SCR 0xDF8
> > +#define SMMU_PMCG_CFGR 0xE00
> > +#define SMMU_PMCG_CFGR_SID_FILTER_TYPE BIT(23)
> > +#define SMMU_PMCG_CFGR_CAPTURE BIT(22)
> > +#define SMMU_PMCG_CFGR_MSI BIT(21)
> > +#define SMMU_PMCG_CFGR_RELOC_CTRS BIT(20)
> > +#define SMMU_PMCG_CFGR_SIZE_MASK GENMASK(13, 8)
> > +#define SMMU_PMCG_CFGR_SIZE_SHIFT 8
> > +#define SMMU_PMCG_CFGR_COUNTER_SIZE_32 31
> > +#define SMMU_PMCG_CFGR_NCTR_MASK GENMASK(5, 0)
> > +#define SMMU_PMCG_CFGR_NCTR_SHIFT 0
> > +#define SMMU_PMCG_CR 0xE04
> > +#define SMMU_PMCG_CR_ENABLE BIT(0)
> > +#define SMMU_PMCG_CEID0 0xE20
> > +#define SMMU_PMCG_CEID1 0xE28
> > +#define SMMU_PMCG_IRQ_CTRL 0xE50
> > +#define SMMU_PMCG_IRQ_CTRL_IRQEN BIT(0)
> > +#define SMMU_PMCG_IRQ_CTRLACK 0xE54
> > +#define SMMU_PMCG_IRQ_CTRLACK_IRQEN BIT(0)
> > +#define SMMU_PMCG_IRQ_CFG0 0xE58
> > +#define SMMU_PMCG_IRQ_CFG0_ADDR_MASK GENMASK(51, 2)
> > +#define SMMU_PMCG_IRQ_CFG1 0xE60
> > +#define SMMU_PMCG_IRQ_CFG2 0xE64
> > +#define SMMU_PMCG_IRQ_STATUS 0xE68
> > +#define SMMU_PMCG_IRQ_STATUS_IRQ_ABT BIT(0)
> > +#define SMMU_PMCG_AIDR 0xE70
>
> Several of these are unused (although at least IRQ0_CFG1 probably should
> be, to zero it to a known state if we aren't supporting MSIs).
>
> > +#define SMMU_COUNTER_RELOAD BIT(31)
> > +#define SMMU_DEFAULT_FILTER_SEC 0
> > +#define SMMU_DEFAULT_FILTER_SPAN 1
> > +#define SMMU_DEFAULT_FILTER_STREAM_ID GENMASK(31, 0)
> > +
> > +#define SMMU_MAX_COUNTERS 64
> > +#define SMMU_MAX_EVENT_ID 128
> > +#define SMMU_NUM_EVENTS_U32 (SMMU_MAX_EVENT_ID / sizeof(u32))
> > +
> > +#define SMMU_PA_SHIFT 12
> > +
> > +/* Events */
> > +#define SMMU_PMU_CYCLES 0
> > +#define SMMU_PMU_TRANSACTION 1
> > +#define SMMU_PMU_TLB_MISS 2
> > +#define SMMU_PMU_CONFIG_CACHE_MISS 3
> > +#define SMMU_PMU_TRANS_TABLE_WALK 4
> > +#define SMMU_PMU_CONFIG_STRUCT_ACCESS 5
> > +#define SMMU_PMU_PCIE_ATS_TRANS_RQ 6
> > +#define SMMU_PMU_PCIE_ATS_TRANS_PASSED 7
> > +
> > +static int cpuhp_state_num;
> > +
> > +struct smmu_pmu {
> > + struct hlist_node node;
> > + struct perf_event *events[SMMU_MAX_COUNTERS];
> > + DECLARE_BITMAP(used_counters, SMMU_MAX_COUNTERS);
> > + DECLARE_BITMAP(supported_events, SMMU_MAX_EVENT_ID);
> > + unsigned int irq;
> > + unsigned int on_cpu;
> > + struct pmu pmu;
> > + unsigned int num_counters;
> > + struct platform_device *pdev;
> > + void __iomem *reg_base;
> > + void __iomem *reloc_base;
> > + u64 counter_present_mask;
> > + u64 counter_mask;
> > + bool reg_size_32;
>
> This guy is redundant...
>
> > +};
> > +
> > +#define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
> > +
> > +#define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _size, _shift) \
> > + static inline u32 get_##_name(struct perf_event *event) \
> > + { \
> > + return (event->attr._config >> (_shift)) & \
> > + GENMASK_ULL((_size) - 1, 0); \
> > + }
> > +
> > +SMMU_PMU_EVENT_ATTR_EXTRACTOR(event, config, 16, 0);
> > +SMMU_PMU_EVENT_ATTR_EXTRACTOR(filter_stream_id, config1, 32, 0);
> > +SMMU_PMU_EVENT_ATTR_EXTRACTOR(filter_span, config1, 1, 32);
> > +SMMU_PMU_EVENT_ATTR_EXTRACTOR(filter_sec, config1, 1, 33);
> > +SMMU_PMU_EVENT_ATTR_EXTRACTOR(filter_enable, config1, 1, 34);
> > +
> > +static inline void smmu_pmu_enable(struct pmu *pmu)
> > +{
> > + struct smmu_pmu *smmu_pmu = to_smmu_pmu(pmu);
> > +
> > + writel(SMMU_PMCG_CR_ENABLE, smmu_pmu->reg_base + SMMU_PMCG_CR);
> > + writel(SMMU_PMCG_IRQ_CTRL_IRQEN,
> > + smmu_pmu->reg_base + SMMU_PMCG_IRQ_CTRL);
> > +}
> > +
> > +static inline void smmu_pmu_disable(struct pmu *pmu)
> > +{
> > + struct smmu_pmu *smmu_pmu = to_smmu_pmu(pmu);
> > +
> > + writel(0, smmu_pmu->reg_base + SMMU_PMCG_CR);
> > + writel(0, smmu_pmu->reg_base + SMMU_PMCG_IRQ_CTRL);
> > +}
> > +
> > +static inline void smmu_pmu_counter_set_value(struct smmu_pmu *smmu_pmu,
> > + u32 idx, u64 value)
> > +{
> > + if (smmu_pmu->reg_size_32)
>
> ...since it would be just as efficient to directly test
> smmu_pmu->counter_mask & BIT(32) here and below.
>
> > + writel(value, smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 4));
> > + else
> > + writeq(value, smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 8));
> > +}
> > +
> > +static inline u64 smmu_pmu_counter_get_value(struct smmu_pmu *smmu_pmu, u32 idx)
> > +{
> > + u64 value;
> > +
> > + if (smmu_pmu->reg_size_32)
> > + value = readl(smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 4));
> > + else
> > + value = readq(smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 8));
> > +
> > + return value;
> > +}
> > +
> > +static inline void smmu_pmu_counter_enable(struct smmu_pmu *smmu_pmu, u32 idx)
> > +{
> > + writeq(BIT(idx), smmu_pmu->reg_base + SMMU_PMCG_CNTENSET0);
> > +}
> > +
> > +static inline void smmu_pmu_counter_disable(struct smmu_pmu *smmu_pmu, u32 idx)
> > +{
> > + writeq(BIT(idx), smmu_pmu->reg_base + SMMU_PMCG_CNTENCLR0);
> > +}
> > +
> > +static inline void smmu_pmu_interrupt_enable(struct smmu_pmu *smmu_pmu, u32 idx)
> > +{
> > + writeq(BIT(idx), smmu_pmu->reg_base + SMMU_PMCG_INTENSET0);
> > +}
> > +
> > +static inline void smmu_pmu_interrupt_disable(struct smmu_pmu *smmu_pmu,
> > + u32 idx)
> > +{
> > + writeq(BIT(idx), smmu_pmu->reg_base + SMMU_PMCG_INTENCLR0);
> > +}
> > +
> > +static void smmu_pmu_reset(struct smmu_pmu *smmu_pmu)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < smmu_pmu->num_counters; i++) {
> > + smmu_pmu_counter_disable(smmu_pmu, i);
> > + smmu_pmu_interrupt_disable(smmu_pmu, i);
> > + }
>
> Surely it would be far quicker and simpler to do this?
>
> writeq(smmu_pmu->counter_present_mask,
> smmu_pmu->reg_base + SMMU_PMCG_CNTENCLR0);
> writeq(smmu_pmu->counter_present_mask,
> smmu_pmu->reg_base + SMMU_PMCG_INTENCLR0);
>
> > + smmu_pmu_disable(&smmu_pmu->pmu);
> > +}
> > +
> > +static inline void smmu_pmu_set_evtyper(struct smmu_pmu *smmu_pmu, u32 idx,
> > + u32 val)
> > +{
> > + writel(val, smmu_pmu->reg_base + SMMU_PMCG_EVTYPER(idx));
> > +}
> > +
> > +static inline void smmu_pmu_set_smr(struct smmu_pmu *smmu_pmu, u32 idx, u32 val)
> > +{
> > + writel(val, smmu_pmu->reg_base + SMMU_PMCG_SMR(idx));
> > +}
> > +
> > +static inline u64 smmu_pmu_getreset_ovsr(struct smmu_pmu *smmu_pmu)
> > +{
> > + u64 result = readq_relaxed(smmu_pmu->reloc_base + SMMU_PMCG_OVSSET0);
> > +
> > + writeq(result, smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
> > + return result;
> > +}
> > +
> > +static inline bool smmu_pmu_has_overflowed(struct smmu_pmu *smmu_pmu, u64 ovsr)
> > +{
> > + return !!(ovsr & smmu_pmu->counter_present_mask);
> > +}
>
> Personally, I find these helpers abstracting simple reads/writes
> actually make the code harder to follow, especially when they're each
> used a grand total of once. That may well just be me, though.
>
> > +static void smmu_pmu_event_update(struct perf_event *event)
> > +{
> > + struct hw_perf_event *hwc = &event->hw;
> > + struct smmu_pmu *smmu_pmu = to_smmu_pmu(event->pmu);
> > + u64 delta, prev, now;
> > + u32 idx = hwc->idx;
> > +
> > + do {
> > + prev = local64_read(&hwc->prev_count);
> > + now = smmu_pmu_counter_get_value(smmu_pmu, idx);
> > + } while (local64_cmpxchg(&hwc->prev_count, prev, now) != prev);
> > +
> > + /* handle overflow. */
> > + delta = now - prev;
> > + delta &= smmu_pmu->counter_mask;
> > +
> > + local64_add(delta, &event->count);
> > +}
> > +
> > +static void smmu_pmu_set_period(struct smmu_pmu *smmu_pmu,
> > + struct hw_perf_event *hwc)
> > +{
> > + u32 idx = hwc->idx;
> > + u64 new;
> > +
> > + /*
> > + * We limit the max period to half the max counter value of the smallest
> > + * counter size, so that even in the case of extreme interrupt latency
> > + * the counter will (hopefully) not wrap past its initial value.
> > + */
>
> Having once fought to properly understand the underlying logic I despise
> this unhelpfully-vague comment, but that's not your fault ;)
>
> > + new = SMMU_COUNTER_RELOAD;
>
> Given that we *are* following the "use the top counter bit as an
> implicit overflow bit" pattern of arm_pmu, it feels a bit weird to not
> use the actual half-maximum value here (especially since it's easily
> computable from counter_mask). I'm about 85% sure it probably still
> works, but as ever inconsistency breeds uncertainty.
> > + local64_set(&hwc->prev_count, new);
> > + smmu_pmu_counter_set_value(smmu_pmu, idx, new);
> > +}
> > +
> > +static irqreturn_t smmu_pmu_handle_irq(int irq_num, void *data)
> > +{
> > + struct smmu_pmu *smmu_pmu = data;
> > + u64 ovsr;
> > + unsigned int idx;
> > +
> > + ovsr = smmu_pmu_getreset_ovsr(smmu_pmu);
> > + if (!smmu_pmu_has_overflowed(smmu_pmu, ovsr))
>
> You have an architectural guarantee that unimplemented bits of OVSSET0
> are RES0, so checking !ovsr is sufficient.
>
> > + return IRQ_NONE;
> > +
> > + for_each_set_bit(idx, (unsigned long *)&ovsr, smmu_pmu->num_counters) {
> > + struct perf_event *event = smmu_pmu->events[idx];
> > + struct hw_perf_event *hwc;
> > +
> > + if (WARN_ON_ONCE(!event))
> > + continue;
> > +
> > + smmu_pmu_event_update(event);
> > + hwc = &event->hw;
> > +
> > + smmu_pmu_set_period(smmu_pmu, hwc);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static unsigned int smmu_pmu_get_event_idx(struct smmu_pmu *smmu_pmu)
> > +{
> > + unsigned int idx;
> > + unsigned int num_ctrs = smmu_pmu->num_counters;
> > +
> > + idx = find_first_zero_bit(smmu_pmu->used_counters, num_ctrs);
> > + if (idx == num_ctrs)
> > + /* The counters are all in use. */
> > + return -EAGAIN;
> > +
> > + set_bit(idx, smmu_pmu->used_counters);
> > +
> > + return idx;
> > +}
> > +
> > +/*
> > + * Implementation of abstract pmu functionality required by
> > + * the core perf events code.
> > + */
> > +
> > +static int smmu_pmu_event_init(struct perf_event *event)
> > +{
> > + struct hw_perf_event *hwc = &event->hw;
> > + struct perf_event *sibling;
> > + struct smmu_pmu *smmu_pmu;
> > + u32 event_id;
> > +
> > + if (event->attr.type != event->pmu->type)
> > + return -ENOENT;
> > +
> > + smmu_pmu = to_smmu_pmu(event->pmu);
> > +
> > + if (hwc->sample_period) {
> > + dev_dbg_ratelimited(&smmu_pmu->pdev->dev,
> > + "Sampling not supported\n");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + if (event->cpu < 0) {
> > + dev_dbg_ratelimited(&smmu_pmu->pdev->dev,
> > + "Per-task mode not supported\n");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + /* We cannot filter accurately so we just don't allow it. */
> > + if (event->attr.exclude_user || event->attr.exclude_kernel ||
> > + event->attr.exclude_hv || event->attr.exclude_idle) {
> > + dev_dbg_ratelimited(&smmu_pmu->pdev->dev,
> > + "Can't exclude execution levels\n");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + /* Verify specified event is supported on this PMU */
> > + event_id = get_event(event);
> > + if ((event_id >= SMMU_MAX_EVENT_ID) ||
>
> What about raw imp-def events?
>
> > + (!test_bit(event_id, smmu_pmu->supported_events))) {
> > + dev_dbg_ratelimited(&smmu_pmu->pdev->dev,
> > + "Invalid event %d for this PMU\n",
> > + event_id);
> > + return -EINVAL;
> > + }
> > +
> > + /* 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_dbg_ratelimited(&smmu_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_dbg_ratelimited(&smmu_pmu->pdev->dev,
> > + "Can't create mixed PMU group\n");
> > + return -EINVAL;
> > + }
> > +
> > + /* Ensure all events in a group are on the same cpu */
> > + if ((event->group_leader != event) &&
> > + (event->cpu != event->group_leader->cpu)) {
> > + dev_dbg_ratelimited(&smmu_pmu->pdev->dev,
> > + "Can't create group on CPUs %d and %d",
> > + event->cpu, event->group_leader->cpu);
> > + return -EINVAL;
> > + }
> > +
> > + hwc->idx = -1;
> > +
> > + /*
> > + * Ensure all events are on the same cpu so all events are in the
> > + * same cpu context, to avoid races on pmu_enable etc.
> > + */
> > + event->cpu = smmu_pmu->on_cpu;
> > +
> > + return 0;
> > +}
> > +
> > +static void smmu_pmu_event_start(struct perf_event *event, int flags)
> > +{
> > + struct smmu_pmu *smmu_pmu = to_smmu_pmu(event->pmu);
> > + struct hw_perf_event *hwc = &event->hw;
> > + int idx = hwc->idx;
> > + u32 evtyper;
> > + u32 filter_sec;
> > + u32 filter_span;
> > + u32 filter_stream_id;
> > +
> > + hwc->state = 0;
> > +
> > + smmu_pmu_set_period(smmu_pmu, hwc);
> > +
> > + if (get_filter_enable(event)) {
> > + filter_sec = get_filter_sec(event);
> > + filter_span = get_filter_span(event);
> > + filter_stream_id = get_filter_stream_id(event);
> > + } else {
> > + filter_sec = SMMU_DEFAULT_FILTER_SEC;
> > + filter_span = SMMU_DEFAULT_FILTER_SPAN;
> > + filter_stream_id = SMMU_DEFAULT_FILTER_STREAM_ID;
> > + }
> > +
> > + evtyper = get_event(event) |
> > + filter_span << SMMU_PMCG_EVTYPER_SID_SPAN_SHIFT |
> > + filter_sec << SMMU_PMCG_EVTYPER_SEC_SID_SHIFT;
> > +
> > + smmu_pmu_set_evtyper(smmu_pmu, idx, evtyper);
> > + smmu_pmu_set_smr(smmu_pmu, idx, filter_stream_id);
> > + smmu_pmu_interrupt_enable(smmu_pmu, idx);
> > + smmu_pmu_counter_enable(smmu_pmu, idx);
> > +}
> > +
> > +static void smmu_pmu_event_stop(struct perf_event *event, int flags)
> > +{
> > + struct smmu_pmu *smmu_pmu = to_smmu_pmu(event->pmu);
> > + struct hw_perf_event *hwc = &event->hw;
> > + int idx = hwc->idx;
> > +
> > + if (hwc->state & PERF_HES_STOPPED)
> > + return;
> > +
> > + smmu_pmu_interrupt_disable(smmu_pmu, idx);
> > + smmu_pmu_counter_disable(smmu_pmu, idx);
> > +
> > + if (flags & PERF_EF_UPDATE)
> > + smmu_pmu_event_update(event);
> > + hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
> > +}
> > +
> > +static int smmu_pmu_event_add(struct perf_event *event, int flags)
> > +{
> > + struct hw_perf_event *hwc = &event->hw;
> > + int idx;
> > + struct smmu_pmu *smmu_pmu = to_smmu_pmu(event->pmu);
> > +
> > + idx = smmu_pmu_get_event_idx(smmu_pmu);
> > + if (idx < 0)
> > + return idx;
> > +
> > + hwc->idx = idx;
> > + hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
> > + smmu_pmu->events[idx] = event;
> > + local64_set(&hwc->prev_count, 0);
> > +
> > + if (flags & PERF_EF_START)
> > + smmu_pmu_event_start(event, flags);
> > +
> > + /* Propagate changes to the userspace mapping. */
> > + perf_event_update_userpage(event);
> > +
> > + return 0;
> > +}
> > +
> > +static void smmu_pmu_event_del(struct perf_event *event, int flags)
> > +{
> > + struct hw_perf_event *hwc = &event->hw;
> > + struct smmu_pmu *smmu_pmu = to_smmu_pmu(event->pmu);
> > + int idx = hwc->idx;
> > +
> > + smmu_pmu_event_stop(event, flags | PERF_EF_UPDATE);
> > + smmu_pmu->events[idx] = NULL;
> > + clear_bit(idx, smmu_pmu->used_counters);
> > +
> > + perf_event_update_userpage(event);
> > +}
> > +
> > +static void smmu_pmu_event_read(struct perf_event *event)
> > +{
> > + smmu_pmu_event_update(event);
> > +}
> > +
> > +/* cpumask */
> > +
> > +static ssize_t smmu_pmu_cpumask_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct smmu_pmu *smmu_pmu = to_smmu_pmu(dev_get_drvdata(dev));
> > +
> > + return cpumap_print_to_pagebuf(true, buf, cpumask_of(smmu_pmu->on_cpu));
> > +}
> > +
> > +static struct device_attribute smmu_pmu_cpumask_attr =
> > + __ATTR(cpumask, 0444, smmu_pmu_cpumask_show, NULL);
> > +
> > +static struct attribute *smmu_pmu_cpumask_attrs[] = {
> > + &smmu_pmu_cpumask_attr.attr,
> > + NULL,
> > +};
> > +
> > +static struct attribute_group smmu_pmu_cpumask_group = {
> > + .attrs = smmu_pmu_cpumask_attrs,
> > +};
> > +
> > +/* Events */
> > +
> > +ssize_t smmu_pmu_event_show(struct device *dev,
> > + struct device_attribute *attr, char *page)
> > +{
> > + struct perf_pmu_events_attr *pmu_attr;
> > +
> > + pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr);
> > +
> > + return sprintf(page, "event=0x%02llx\n", pmu_attr->id);
> > +}
> > +
> > +#define SMMU_EVENT_ATTR(_name, _id) \
> > + (&((struct perf_pmu_events_attr[]) { \
> > + { .attr = __ATTR(_name, 0444, smmu_pmu_event_show, NULL), \
> > + .id = _id, } \
> > + })[0].attr.attr)
> > +
> > +static struct attribute *smmu_pmu_events[] = {
> > + SMMU_EVENT_ATTR(cycles, SMMU_PMU_CYCLES),
> > + SMMU_EVENT_ATTR(transaction, SMMU_PMU_TRANSACTION),
> > + SMMU_EVENT_ATTR(tlb_miss, SMMU_PMU_TLB_MISS),
> > + SMMU_EVENT_ATTR(config_cache_miss, SMMU_PMU_CONFIG_CACHE_MISS),
> > + SMMU_EVENT_ATTR(trans_table_walk, SMMU_PMU_TRANS_TABLE_WALK),
> > + SMMU_EVENT_ATTR(config_struct_access, SMMU_PMU_CONFIG_STRUCT_ACCESS),
> > + SMMU_EVENT_ATTR(pcie_ats_trans_rq, SMMU_PMU_PCIE_ATS_TRANS_RQ),
> > + SMMU_EVENT_ATTR(pcie_ats_trans_passed, SMMU_PMU_PCIE_ATS_TRANS_PASSED),
> > + NULL
> > +};
> > +
> > +static umode_t smmu_pmu_event_is_visible(struct kobject *kobj,
> > + struct attribute *attr, int unused)
> > +{
> > + struct device *dev = kobj_to_dev(kobj);
> > + struct smmu_pmu *smmu_pmu = to_smmu_pmu(dev_get_drvdata(dev));
> > + struct perf_pmu_events_attr *pmu_attr;
> > +
> > + pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr.attr);
> > +
> > + if (test_bit(pmu_attr->id, smmu_pmu->supported_events))
> > + return attr->mode;
> > +
> > + return 0;
> > +}
> > +static struct attribute_group smmu_pmu_events_group = {
> > + .name = "events",
> > + .attrs = smmu_pmu_events,
> > + .is_visible = smmu_pmu_event_is_visible,
> > +};
> > +
> > +/* Formats */
> > +PMU_FORMAT_ATTR(event, "config:0-15");
> > +PMU_FORMAT_ATTR(filter_stream_id, "config1:0-31");
> > +PMU_FORMAT_ATTR(filter_span, "config1:32");
> > +PMU_FORMAT_ATTR(filter_sec, "config1:33");
> > +PMU_FORMAT_ATTR(filter_enable, "config1:34");
> > +
> > +static struct attribute *smmu_pmu_formats[] = {
> > + &format_attr_event.attr,
> > + &format_attr_filter_stream_id.attr,
> > + &format_attr_filter_span.attr,
> > + &format_attr_filter_sec.attr,
> > + &format_attr_filter_enable.attr,
> > + NULL
> > +};
> > +
> > +static struct attribute_group smmu_pmu_format_group = {
> > + .name = "format",
> > + .attrs = smmu_pmu_formats,
> > +};
> > +
> > +static const struct attribute_group *smmu_pmu_attr_grps[] = {
> > + &smmu_pmu_cpumask_group,
> > + &smmu_pmu_events_group,
> > + &smmu_pmu_format_group,
> > + NULL,
> > +};
> > +
> > +/*
> > + * Generic device handlers
> > + */
> > +
> > +static unsigned int get_num_counters(struct smmu_pmu *smmu_pmu)
> > +{
> > + u32 cfgr = readl(smmu_pmu->reg_base + SMMU_PMCG_CFGR);
> > +
> > + return ((cfgr & SMMU_PMCG_CFGR_NCTR_MASK) >> SMMU_PMCG_CFGR_NCTR_SHIFT)
> > + + 1;
> > +}
> > +
> > +static int smmu_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> > +{
> > + struct smmu_pmu *smmu_pmu;
> > + unsigned int target;
> > +
> > + smmu_pmu = hlist_entry_safe(node, struct smmu_pmu, node);
>
> Is it ever valid for node to be NULL? If we can't trust it to be one of
> the PMUs we registered I think all bets are off anyway.
>
> > + if (cpu != smmu_pmu->on_cpu)
> > + return 0;
> > +
> > + target = cpumask_any_but(cpu_online_mask, cpu);
> > + if (target >= nr_cpu_ids)
> > + return 0;
> > +
> > + perf_pmu_migrate_context(&smmu_pmu->pmu, cpu, target);
> > + smmu_pmu->on_cpu = target;
> > + WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(target)));
> > +
> > + return 0;
> > +}
> > +
> > +static int smmu_pmu_probe(struct platform_device *pdev)
> > +{
> > + struct smmu_pmu *smmu_pmu;
> > + struct resource *mem_resource_0, *mem_resource_1;
> > + void __iomem *mem_map_0, *mem_map_1;
> > + unsigned int reg_size;
> > + int err;
> > + int irq;
> > + u32 ceid[SMMU_NUM_EVENTS_U32];
> > + u64 ceid_64;
> > +
> > + smmu_pmu = devm_kzalloc(&pdev->dev, sizeof(*smmu_pmu), GFP_KERNEL);
> > + if (!smmu_pmu)
> > + return -ENOMEM;
> > +
> > + platform_set_drvdata(pdev, smmu_pmu);
> > + smmu_pmu->pmu = (struct pmu) {
> > + .task_ctx_nr = perf_invalid_context,
> > + .pmu_enable = smmu_pmu_enable,
> > + .pmu_disable = smmu_pmu_disable,
> > + .event_init = smmu_pmu_event_init,
> > + .add = smmu_pmu_event_add,
> > + .del = smmu_pmu_event_del,
> > + .start = smmu_pmu_event_start,
> > + .stop = smmu_pmu_event_stop,
> > + .read = smmu_pmu_event_read,
> > + .attr_groups = smmu_pmu_attr_grps,
> > + };
> > +
> > + mem_resource_0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + mem_map_0 = devm_ioremap_resource(&pdev->dev, mem_resource_0);
> > +
> > + if (IS_ERR(mem_map_0)) {
> > + dev_err(&pdev->dev, "Can't map SMMU PMU @%pa\n",
> > + &mem_resource_0->start);
> > + return PTR_ERR(mem_map_0);
> > + }
> > +
> > + smmu_pmu->reg_base = mem_map_0;
> > + smmu_pmu->pmu.name =
> > + devm_kasprintf(&pdev->dev, GFP_KERNEL, "smmu_0_%llx",
> > + (mem_resource_0->start) >> SMMU_PA_SHIFT);
> > +
> > + if (!smmu_pmu->pmu.name) {
> > + dev_err(&pdev->dev, "Failed to create PMU name");
> > + return -EINVAL;
> > + }
> > +
> > + ceid_64 = readq(smmu_pmu->reg_base + SMMU_PMCG_CEID0);
> > + ceid[0] = ceid_64 & GENMASK(31, 0);
>
> It took a second look to determine that that masking does nothing...
>
> > + ceid[1] = ceid_64 >> 32;
> > + ceid_64 = readq(smmu_pmu->reg_base + SMMU_PMCG_CEID1);
> > + ceid[2] = ceid_64 & GENMASK(31, 0);
> > + ceid[3] = ceid_64 >> 32;
> > + bitmap_from_u32array(smmu_pmu->supported_events, SMMU_MAX_EVENT_ID,
> > + ceid, SMMU_NUM_EVENTS_U32);
>
> ...but then the whole lot might be cleaner and simpler with a u64[2]
> cast to u32* (or unioned to u32[4]) as necessary.
>
> > +
> > + /* Determine if page 1 is present */
> > + if (readl(smmu_pmu->reg_base + SMMU_PMCG_CFGR) &
> > + SMMU_PMCG_CFGR_RELOC_CTRS) {
> > + mem_resource_1 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > + mem_map_1 = devm_ioremap_resource(&pdev->dev, mem_resource_1);
> > +
> > + if (IS_ERR(mem_map_1)) {
> > + dev_err(&pdev->dev, "Can't map SMMU PMU @%pa\n",
> > + &mem_resource_1->start);
> > + return PTR_ERR(mem_map_1);
> > + }
> > + smmu_pmu->reloc_base = mem_map_1;
> > + } else {
> > + smmu_pmu->reloc_base = smmu_pmu->reg_base;
> > + }
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0) {
> > + dev_err(&pdev->dev,
> > + "Failed to get valid irq for smmu @%pa\n",
> > + &mem_resource_0->start);
> > + return irq;
> > + }
> > +
> > + err = devm_request_irq(&pdev->dev, irq, smmu_pmu_handle_irq,
> > + IRQF_NOBALANCING | IRQF_SHARED | IRQF_NO_THREAD,
> > + "smmu-pmu", smmu_pmu);
> > + if (err) {
> > + dev_err(&pdev->dev,
> > + "Unable to request IRQ%d for SMMU PMU counters\n", irq);
> > + return err;
> > + }
> > +
> > + smmu_pmu->irq = irq;
> > +
> > + /* Pick one CPU to be the preferred one to use */
> > + smmu_pmu->on_cpu = smp_processor_id();
> > + WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu->on_cpu)));
> > +
> > + smmu_pmu->num_counters = get_num_counters(smmu_pmu);
> > + smmu_pmu->pdev = pdev;
> > + smmu_pmu->counter_present_mask = GENMASK(smmu_pmu->num_counters - 1, 0);
> > + reg_size = (readl(smmu_pmu->reg_base + SMMU_PMCG_CFGR) &
> > + SMMU_PMCG_CFGR_SIZE_MASK) >> SMMU_PMCG_CFGR_SIZE_SHIFT;
> > + smmu_pmu->reg_size_32 = (reg_size == SMMU_PMCG_CFGR_COUNTER_SIZE_32);
> > + smmu_pmu->counter_mask = GENMASK_ULL(reg_size, 0);
> > +
> > + smmu_pmu_reset(smmu_pmu);
> > +
> > + err = cpuhp_state_add_instance_nocalls(cpuhp_state_num,
> > + &smmu_pmu->node);
> > + if (err) {
> > + dev_err(&pdev->dev, "Error %d registering hotplug", err);
> > + return err;
> > + }
> > +
> > + err = perf_pmu_register(&smmu_pmu->pmu, smmu_pmu->pmu.name, -1);
> > + if (err) {
> > + dev_err(&pdev->dev, "Error %d registering SMMU PMU\n", err);
> > + goto out_unregister;
> > + }
> > +
> > + dev_info(&pdev->dev, "Registered SMMU PMU @ %pa using %d counters\n",
> > + &mem_resource_0->start, smmu_pmu->num_counters);
> > +
> > + return err;
> > +
> > +out_unregister:
> > + cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node);
> > + return err;
> > +}
> > +
> > +static int smmu_pmu_remove(struct platform_device *pdev)
> > +{
> > + struct smmu_pmu *smmu_pmu = platform_get_drvdata(pdev);
> > +
> > + perf_pmu_unregister(&smmu_pmu->pmu);
> > + cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node);
> > +
> > + return 0;
> > +}
> > +
> > +static void smmu_pmu_shutdown(struct platform_device *pdev)
> > +{
> > + struct smmu_pmu *smmu_pmu = platform_get_drvdata(pdev);
> > +
> > + smmu_pmu_disable(&smmu_pmu->pmu);
> > +}
> > +
> > +static struct platform_driver smmu_pmu_driver = {
> > + .driver = {
> > + .name = "arm-smmu-pmu",
>
> Nit: "arm-smmu-v3-pmu" please, for consistency with the IOMMU driver
> naming. There is a SMMUv2 PMU driver in the works, too ;)
>
> Robin.
>
> > + },
> > + .probe = smmu_pmu_probe,
> > + .remove = smmu_pmu_remove,
> > + .shutdown = smmu_pmu_shutdown,
> > +};
> > +
> > +static int __init arm_smmu_pmu_init(void)
> > +{
> > + cpuhp_state_num = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
> > + "perf/arm/smmupmu:online",
> > + NULL,
> > + smmu_pmu_offline_cpu);
> > + if (cpuhp_state_num < 0)
> > + return cpuhp_state_num;
> > +
> > + return platform_driver_register(&smmu_pmu_driver);
> > +}
> > +module_init(arm_smmu_pmu_init);
> > +
> > +static void __exit arm_smmu_pmu_exit(void)
> > +{
> > + platform_driver_unregister(&smmu_pmu_driver);
> > +}
> > +
> > +module_exit(arm_smmu_pmu_exit);
> > +MODULE_LICENSE("GPL v2");
> >
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Linu cherian
^ permalink raw reply
* [RFC PATCH] arm64: deactivate saved ttbr when mm is deactivated
From: Vinayak Menon @ 2017-12-05 5:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171204180001.7gsub3wt6h5nl3v3@lakrids.cambridge.arm.com>
On 12/4/2017 11:30 PM, Mark Rutland wrote:
> On Mon, Dec 04, 2017 at 04:55:33PM +0000, Will Deacon wrote:
>> On Mon, Dec 04, 2017 at 09:53:26PM +0530, Vinayak Menon wrote:
>>> A case is observed where a wrong physical address is read,
>>> resulting in a bus error and that happens soon after TTBR0 is
>>> set to the saved ttbr by uaccess_ttbr0_enable. This is always
>>> seen to happen in the exit path of the task.
>>>
>>> exception
>>> __arch_copy_from_user
>>> __copy_from_user
>>> probe_kernel_read
>>> get_freepointer_safe
>>> slab_alloc_node
>>> slab_alloc
>>> kmem_cache_alloc
>>> kmem_cache_zalloc
>>> fill_pool
>>> __debug_object_init
>>> debug_object_init
>>> rcuhead_fixup_activate
>>> debug_object_fixup
>>> debug_object_activate
>>> debug_rcu_head_queue
>>> __call_rcu
>>> ep_remove
>>> eventpoll_release_file
>>> __fput
>>> ____fput
>>> task_work_run
>>> do_exit
>>>
>>> The mm has been released and the pgd is freed, but probe_kernel_read
>>> invoked from slub results in call to __arch_copy_from_user. At the
>>> entry to __arch_copy_from_user, when SW PAN is enabled, this results
>>> in stale value being set to ttbr0. May be a speculative fetch aftwerwards
>>> is resulting in invalid physical address access.
>>>
>>> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
>>> ---
>>>
>>> I have not tested this patch to see if it fixes the problem.
>>> Sending it early for comments.
>> I wonder whether it would be better to avoid restoring the user TTBR0 if
>> KERNEL_DS is set.
> I think the problem here is that switch_mm() avoids updating the saved ttbr
> value when the next mm is init_mm.
For this switch to happen, the schedule() in do_task_dead at the end of do_exit() need to be called, right ?
The issue is happening soon after exit_mm (probably from exit_files).
>
> If we fixed that up to use the empty zero page (as we write to the real
> ttbr0 in this case), I think that solves the problem. Though I agree we
> should also avoid restoring the user TTBR for KERNEL_DS uaccess calls.
>
> Example below.
>
> Thanks,
> Mark.
>
> ---->8----
> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
> index 3257895a9b5e..ef3567ce80b3 100644
> --- a/arch/arm64/include/asm/mmu_context.h
> +++ b/arch/arm64/include/asm/mmu_context.h
> @@ -174,10 +174,15 @@ enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
> static inline void update_saved_ttbr0(struct task_struct *tsk,
> struct mm_struct *mm)
> {
> + u64 ttbr;
> +
> if (system_uses_ttbr0_pan()) {
> - BUG_ON(mm->pgd == swapper_pg_dir);
> - task_thread_info(tsk)->ttbr0 =
> - virt_to_phys(mm->pgd) | ASID(mm) << 48;
> + if (mm == &init_mm)
> + ttbr = __pa_symbol(empty_zero_page);
> + else
> + ttbr = virt_to_phys(mm->pgd) | ASID(mm) << 48;
> +
> + task_thread_info(tsk)->ttbr0 = ttbr;
> }
> }
> #else
> @@ -214,11 +219,9 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
> * Update the saved TTBR0_EL1 of the scheduled-in task as the previous
> * value may have not been initialised yet (activate_mm caller) or the
> * ASID has changed since the last run (following the context switch
> - * of another thread of the same process). Avoid setting the reserved
> - * TTBR0_EL1 to swapper_pg_dir (init_mm; e.g. via idle_task_exit).
> + * of another thread of the same process).
> */
> - if (next != &init_mm)
> - update_saved_ttbr0(tsk, next);
> + update_saved_ttbr0(tsk, next);
> }
>
> #define deactivate_mm(tsk,mm) do { } while (0)
^ permalink raw reply
* [PATCH 5/8] ASoC: uniphier: add support for UniPhier AIO driver
From: Katsuhiro Suzuki @ 2017-12-05 4:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171204183934.rd4vin22ktukwpip@sirena.org.uk>
Hello,
Thanks a lot for your review.
> -----Original Message-----
> From: Mark Brown [mailto:broonie at kernel.org]
> Sent: Tuesday, December 5, 2017 3:40 AM
> To: Suzuki, Katsuhiro/?? ?? <suzuki.katsuhiro@socionext.com>
> Cc: alsa-devel at alsa-project.org; Rob Herring <robh+dt@kernel.org>; devicetree at vger.kernel.org; Yamada, Masahiro/?
> ? ?? <yamada.masahiro@socionext.com>; Masami Hiramatsu <masami.hiramatsu@linaro.org>; Jassi Brar
> <jaswinder.singh@linaro.org>; linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org
> Subject: Re: [PATCH 5/8] ASoC: uniphier: add support for UniPhier AIO driver
>
> On Wed, Nov 22, 2017 at 08:43:18PM +0900, Katsuhiro Suzuki wrote:
>
> > sound/soc/uniphier/Makefile | 4 +
> > sound/soc/uniphier/aio-core.c | 368 +++++++++++++++++++++
> > sound/soc/uniphier/aio-dma.c | 266 +++++++++++++++
> > sound/soc/uniphier/aio-regctrl.c | 699 +++++++++++++++++++++++++++++++++++++++
> > sound/soc/uniphier/aio-regctrl.h | 495 +++++++++++++++++++++++++++
> > sound/soc/uniphier/aio.h | 261 +++++++++++++++
>
> Please split this up more, it looks like there's at least two or three
> drivers in here and it winds up being quite large. There's at least a
> DMA and a DAI driver. Looking through this my overall impression is
> that this is a fairly large and complex audio subsystem with some DSP
> and routing capacity which is being handled in a board specific fashion
> rather than generically but it's kind of hard to tell as there's not
> much description of what's going on so I'm needing to reverse engineer
> things from the driver.
>
> The code itself looks fairly clean, it's mainly a case of trying to
> figure out if it's doing what it's supposed to with the limited
> documentation.
>
> > +int uniphier_aio_hw_params(struct snd_pcm_substream *substream,
> > + struct snd_pcm_hw_params *params,
> > + struct snd_soc_dai *dai)
> > +{
> > + struct uniphier_aio *aio = uniphier_priv(dai);
> > + struct uniphier_aio_sub *sub = &aio->sub[substream->stream];
> > +
> > + sub->params = *params;
> > + sub->setting = 1;
>
> So we don't validate the params at all?
>
> > + uniphier_aio_port_reset(sub);
> > + uniphier_aio_srcport_reset(sub);
>
> Is there a mux in the SoC here?
>
Sorry for confusing, It's not mux.
uniphier_srcport_reset() resets HW SRC (sampling rate converter) block.
Audio data out ports of UniPhier audio system have HW SRC.
> > +static const struct of_device_id uniphier_aio_of_match[] = {
> > +#ifdef CONFIG_SND_SOC_UNIPHIER_LD11
> > + {
> > + .compatible = "socionext,uniphier-ld11-aio",
> > + .data = &uniphier_aio_ld11_spec,
> > + },
> > + {
> > + .compatible = "socionext,uniphier-ld20-aio",
> > + .data = &uniphier_aio_ld20_spec,
> > + },
> > +#endif /* CONFIG_SND_SOC_UNIPHIER_LD11 */
>
> Why is there an ifdef here? There's no other conditional code in here,
> it seems pointless.
>
This config is used to support or not LD11 SoC.
aio-ld11.c is not build and 'uniphier_aio_ldxx_spec' is undefined if this config is disabled.
aio-ld11.c defines SoC dependent resources (port, HW ring buffer, DMA ch, etc.)
and fixed settings.
I know it's better to move such information into device-tree, but register areas of
UniPhier's audio system is very strange and interleaved. It's hard to split each nodes...
> > + for (j = 0; j < ARRAY_SIZE(aio->sub); j++) {
> > + struct uniphier_aio_sub *sub = &aio->sub[j];
> > +
> > + if (!sub->running)
> > + continue;
> > +
> > + spin_lock(&sub->spin);
> > + uniphier_aio_rb_sync(sub);
> > + uniphier_aio_rb_clear_int(sub);
> > + spin_unlock(&sub->spin);
>
> It's not 100% obvious what this does... a comment might help.
>
> > +int uniphier_aio_chip_init(struct uniphier_aio_chip *chip)
> > +{
> > + struct regmap *r = chip->regmap;
> > +
> > + regmap_update_bits(r, A2APLLCTR0,
> > + A2APLLCTR0_APLLXPOW_MASK,
> > + A2APLLCTR0_APLLXPOW_PWON);
> > +
> > + regmap_update_bits(r, A2APLLCTR1, A2APLLCTR1_APLL_MASK,
> > + A2APLLCTR1_APLLF2_33MHZ | A2APLLCTR1_APLLA2_33MHZ |
> > + A2APLLCTR1_APLLF1_36MHZ | A2APLLCTR1_APLLA1_36MHZ);
> > +
> > + regmap_update_bits(r, A2EXMCLKSEL0,
> > + A2EXMCLKSEL0_EXMCLK_MASK,
> > + A2EXMCLKSEL0_EXMCLK_OUTPUT);
> > +
> > + regmap_update_bits(r, A2AIOINPUTSEL, A2AIOINPUTSEL_RXSEL_MASK,
> > + A2AIOINPUTSEL_RXSEL_PCMI1_HDMIRX1 |
> > + A2AIOINPUTSEL_RXSEL_PCMI2_SIF |
> > + A2AIOINPUTSEL_RXSEL_PCMI3_EVEA |
> > + A2AIOINPUTSEL_RXSEL_IECI1_HDMIRX1);
>
> This definitely looks like there's some clocking and audio routing
> within the SoC which should be exposed to userspace, or at the very
> least machine driver configuration rather than being hard coded.
>
> > + switch (pc) {
> > + case IEC61937_PC_AC3:
> > + repet = OPORTMXREPET_STRLENGTH_AC3 |
> > + OPORTMXREPET_PMLENGTH_AC3;
> > + pause |= OPORTMXPAUDAT_PAUSEPD_AC3;
> > + break;
> > + case IEC61937_PC_MPA:
> > + repet = OPORTMXREPET_STRLENGTH_MPA |
> > + OPORTMXREPET_PMLENGTH_MPA;
> > + pause |= OPORTMXPAUDAT_PAUSEPD_MPA;
> > + break;
> > + case IEC61937_PC_MP3:
> > + repet = OPORTMXREPET_STRLENGTH_MP3 |
> > + OPORTMXREPET_PMLENGTH_MP3;
> > + pause |= OPORTMXPAUDAT_PAUSEPD_MP3;
> > + break;
>
> This looks awfully like compressed audio support... should there be
> integration with the compressed audio API/
Thanks, I'll try it. Is there Documentation in sound/designes/compress-offload.rst?
And best sample is... Intel's driver?
(Summary)
I think I should fix as follows:
- Split DMA, DAI patches from large one
- Validate parameters in hw_params
- Add description about HW SRC (or fix bad name 'srcport')
- Add comments about uniphier_aiodma_irq()
- Expose clocking and audio routing to userspace, or at the very
least machine driver configuration
- Support compress-audio API for S/PDIF
and post V2.
Regards,
--
Katsuhiro Suzuki
^ permalink raw reply
* [PATCH RFC 1/2] dt-bindings: Add vendor prefix for Brava Home
From: Jagan Teki @ 2017-12-05 4:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171204213605.vg3rpz7rscbomduv@rob-hp-laptop>
On Tue, Dec 5, 2017 at 3:06 AM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Dec 01, 2017 at 12:12:13AM +0530, Jagan Teki wrote:
>> Added 'brava' as a vendor prefix for Brava Home, Inc.
>> which is consumer electronics and IoT company.
>>
>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>> ---
>> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>> 1 file changed, 1 insertion(+)
>
> Reviewed-by: Rob Herring <robh@kernel.org>
Thanks, please ignore these RFC's as of now.
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
^ permalink raw reply
* [linux-sunxi] [PATCH 1/8] iio: adc: axp20x_adc: put ADC rate setting in a per-variant function
From: Chen-Yu Tsai @ 2017-12-05 3:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e97d7332790facf3d10a58feabbfe7b2c0b90db7.1512396054.git-series.quentin.schulz@free-electrons.com>
On Mon, Dec 4, 2017 at 10:12 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> To prepare for a new comer that set a different register with different
> values, move rate setting in a function that is specific to each AXP
> variant.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> ---
> drivers/iio/adc/axp20x_adc.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index a30a972..7274f4f 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -470,14 +470,18 @@ static const struct iio_info axp22x_adc_iio_info = {
> .read_raw = axp22x_read_raw,
> };
>
> -static int axp20x_adc_rate(int rate)
> +static int axp20x_adc_rate(struct axp20x_adc_iio *info, int rate)
> {
> - return AXP20X_ADC_RATE_HZ(rate);
> + return regmap_update_bits(info->regmap, AXP20X_ADC_RATE,
> + AXP20X_ADC_RATE_MASK,
> + AXP20X_ADC_RATE_HZ(rate));
> }
>
> -static int axp22x_adc_rate(int rate)
> +static int axp22x_adc_rate(struct axp20x_adc_iio *info, int rate)
> {
> - return AXP22X_ADC_RATE_HZ(rate);
> + return regmap_update_bits(info->regmap, AXP20X_ADC_RATE,
> + AXP20X_ADC_RATE_MASK,
> + AXP22X_ADC_RATE_HZ(rate));
> }
>
> struct axp_data {
> @@ -485,7 +489,7 @@ struct axp_data {
> int num_channels;
> struct iio_chan_spec const *channels;
> unsigned long adc_en1_mask;
> - int (*adc_rate)(int rate);
> + int (*adc_rate)(struct axp20x_adc_iio *info, int rate);
Could you also change the name of the callback, to say, adc_set_rate?
This would make it much clearer what the callback does. Previously
it was just a conversion helper.
ChenYu
> bool adc_en2;
> struct iio_map *maps;
> };
> @@ -554,8 +558,7 @@ static int axp20x_probe(struct platform_device *pdev)
> AXP20X_ADC_EN2_MASK, AXP20X_ADC_EN2_MASK);
>
> /* Configure ADCs rate */
> - regmap_update_bits(info->regmap, AXP20X_ADC_RATE, AXP20X_ADC_RATE_MASK,
> - info->data->adc_rate(100));
> + info->data->adc_rate(info, 100);
>
> ret = iio_map_array_register(indio_dev, info->data->maps);
> if (ret < 0) {
> --
> git-series 0.9.1
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [PATCH v3 2/3] mailbox: Add support for Hi3660 mailbox
From: Jassi Brar @ 2017-12-05 3:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1510910672-1409-3-git-send-email-xuyiping@hisilicon.com>
On Fri, Nov 17, 2017 at 2:54 PM, Xu YiPing <xuyiping@hisilicon.com> wrote:
> From: Kaihua Zhong <zhongkaihua@huawei.com>
>
> Hi3660 mailbox controller is used to send message within multiple
> processors, MCU, HIFI, etc. It supports 32 mailbox channels and every
> channel can only be used for single transferring direction. Once the
> channel is enabled, it needs to specify the destination interrupt and
> acknowledge interrupt, these two interrupt vectors are used to create
> the connection between the mailbox and interrupt controllers.
>
> The data transferring supports two modes, one is named as "automatic
> acknowledge" mode so after send message the kernel doesn't need to wait
> for acknowledge from remote and directly return; there have another mode
> is to rely on handling interrupt for acknowledge.
>
> This commit is for initial version driver, which only supports
> "automatic acknowledge" mode to support CPU clock, which is the only
> one consumer to use mailbox and has been verified. Later may enhance
> this driver for interrupt mode (e.g. for supporting HIFI).
>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: Ruyi Wang <wangruyi@huawei.com>
> Signed-off-by: Kaihua Zhong <zhongkaihua@huawei.com>
> ---
> drivers/mailbox/Kconfig | 8 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/hi3660-mailbox.c | 322 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 332 insertions(+)
> create mode 100644 drivers/mailbox/hi3660-mailbox.c
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index c5731e5..4b5d6e9 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -108,6 +108,14 @@ config TI_MESSAGE_MANAGER
> multiple processors within the SoC. Select this driver if your
> platform has support for the hardware block.
>
> +config HI3660_MBOX
> + tristate "Hi3660 Mailbox"
> + depends on ARCH_HISI && OF
> + help
> + An implementation of the hi3660 mailbox. It is used to send message
> + between application processors and other processors/MCU/DSP. Select
> + Y here if you want to use Hi3660 mailbox controller.
> +
> config HI6220_MBOX
> tristate "Hi6220 Mailbox"
> depends on ARCH_HISI
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index d54e412..7d1bd51 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o
>
> obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o
>
> +obj-$(CONFIG_HI3660_MBOX) += hi3660-mailbox.o
> +
> obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o
>
> obj-$(CONFIG_BCM_PDC_MBOX) += bcm-pdc-mailbox.o
> diff --git a/drivers/mailbox/hi3660-mailbox.c b/drivers/mailbox/hi3660-mailbox.c
> new file mode 100644
> index 0000000..ba80834
> --- /dev/null
> +++ b/drivers/mailbox/hi3660-mailbox.c
> @@ -0,0 +1,322 @@
> +/*
> + * Hisilicon's Hi3660 mailbox controller driver
> + *
> + * Copyright (c) 2017 Hisilicon Limited.
> + * Copyright (c) 2017 Linaro Limited.
> + *
> + * Author: Leo Yan <leo.yan@linaro.org>
> + *
> + * This program 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, version 2 of the License.
> + *
> + * This program 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.
> + *
>
We now use SPDX licence
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "mailbox.h"
> +
> +#define MBOX_CHAN_MAX 32
> +
> +#define MBOX_RX (0x0)
> +#define MBOX_TX (0x1)
> +
> +#define MBOX_BASE(mbox, ch) ((mbox)->base + ((ch) * 0x40))
> +#define MBOX_SRC_REG (0x00)
> +#define MBOX_DST_REG (0x04)
> +#define MBOX_DCLR_REG (0x08)
> +#define MBOX_DSTAT_REG (0x0c)
> +#define MBOX_MODE_REG (0x10)
> +#define MBOX_IMASK_REG (0x14)
> +#define MBOX_ICLR_REG (0x18)
> +#define MBOX_SEND_REG (0x1c)
> +#define MBOX_DATA_REG (0x20)
> +
> +#define MBOX_IPC_LOCK_REG (0xa00)
> +#define MBOX_IPC_UNLOCK (0x1acce551)
> +
> +#define MBOX_AUTOMATIC_ACK (1)
> +
Please, no brackets around numbers.
> +#define MBOX_STATE_IDLE BIT(4)
> +#define MBOX_STATE_ACK BIT(7)
> +
> +#define MBOX_MSG_LEN 8
> +
> +/**
> + * Hi3660 mailbox channel device data
> + *
> + * A channel can be used for TX or RX, it can trigger remote
> + * processor interrupt to notify remote processor and can receive
> + * interrupt if has incoming message.
> + *
> + * @dst_irq: Interrupt vector for remote processor
> + * @ack_irq: Interrupt vector for local processor
> + */
> +struct hi3660_mbox_dev {
>
Better than dev, maybe hi3660_chan_info ?
> + unsigned int dst_irq;
> + unsigned int ack_irq;
> +};
> +
> +/**
> + * Hi3660 mailbox controller data
> + *
> + * Mailbox controller includes 32 channels and can allocate
> + * channel for message transferring.
> + *
> + * @dev: Device to which it is attached
> + * @base: Base address of the register mapping region
> + * @chan: Representation of channels in mailbox controller
> + * @mdev: Representation of channel device data
> + * @controller: Representation of a communication channel controller
> + */
> +struct hi3660_mbox {
> + struct device *dev;
> + void __iomem *base;
> + struct mbox_chan chan[MBOX_CHAN_MAX];
> + struct hi3660_mbox_dev mdev[MBOX_CHAN_MAX];
>
Maybe mchan, instead of mdev.
> + struct mbox_controller controller;
> +};
> +
> +static inline struct hi3660_mbox *to_hi3660_mbox(struct mbox_controller *mbox)
> +{
> + return container_of(mbox, struct hi3660_mbox, controller);
> +}
> +
> +static int hi3660_mbox_check_state(struct mbox_chan *chan)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned long val;
> + unsigned int state, ret;
> +
> + /* Mailbox is idle so directly bail out */
> + state = readl_relaxed(base + MBOX_MODE_REG);
> + if (state & MBOX_STATE_IDLE)
> + return 0;
> +
> + /* Wait for acknowledge from remote */
> + ret = readx_poll_timeout_atomic(readl_relaxed, base + MBOX_MODE_REG,
> + val, (val & MBOX_STATE_ACK), 1000, 300000);
> + if (ret) {
> + dev_err(mbox->dev, "%s: timeout for receiving ack\n", __func__);
> + return ret;
> + }
> +
> + /* Ensure channel is released */
> + writel_relaxed(0xffffffff, base + MBOX_IMASK_REG);
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> +
> + return 0;
> +}
> +
> +static int hi3660_mbox_unlock(struct mbox_chan *chan)
> +{
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + unsigned int val, retry = 3;
> +
> + do {
> + writel_relaxed(MBOX_IPC_UNLOCK, mbox->base + MBOX_IPC_LOCK_REG);
> +
> + val = readl_relaxed(mbox->base + MBOX_IPC_LOCK_REG);
> + if (!val)
> + break;
> +
> + udelay(10);
> + } while (retry--);
> +
> + return (!val) ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_acquire_channel(struct mbox_chan *chan)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned int val, retry;
> +
> + for (retry = 10; retry; retry--) {
> + /* Check if channel has been acquired */
> + if (readl_relaxed(base + MBOX_MODE_REG) & MBOX_STATE_IDLE) {
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> + val = readl_relaxed(base + MBOX_SRC_REG);
> + if (val & BIT(mdev->ack_irq))
> + break;
> + }
> + }
> +
> + return retry ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_send(struct mbox_chan *chan, u32 *msg)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned int i;
> +
> + /* Clear mask for destination interrupt */
> + writel_relaxed(~BIT(mdev->dst_irq), base + MBOX_IMASK_REG);
> +
> + /* Config destination for interrupt vector */
> + writel_relaxed(BIT(mdev->dst_irq), base + MBOX_DST_REG);
> +
> + /* Automatic acknowledge mode */
> + writel_relaxed(MBOX_AUTOMATIC_ACK, base + MBOX_MODE_REG);
> +
> + /* Fill message data */
> + for (i = 0; i < MBOX_MSG_LEN; i++)
> + writel_relaxed(msg[i], base + MBOX_DATA_REG + i * 4);
> +
> + /* Trigger data transferring */
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SEND_REG);
> + return 0;
> +}
> +
> +static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)
> +{
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + int err;
> +
> + err = hi3660_mbox_check_state(chan);
> + if (err) {
> + dev_err(mbox->dev, "checking state failed\n");
> + return err;
> + }
> +
> + err = hi3660_mbox_unlock(chan);
> + if (err) {
> + dev_err(mbox->dev, "unlocking mailbox failed\n");
> + return err;
> + }
> +
> + err = hi3660_mbox_acquire_channel(chan);
> + if (err) {
> + dev_err(mbox->dev, "acquiring channel failed\n");
> + return err;
> + }
> +
Please remember .send_data() is called with irqs disabled spinlock
All these polling functions hurt. Please move the above three checks
in last_tx_done()
Thanks
^ permalink raw reply
* [PATCH 9/9] arm64: dts: allwinner: a64: Enable sound on Pine64 and SoPine
From: Chen-Yu Tsai @ 2017-12-05 3:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171203204157.20829-10-anarsoul@gmail.com>
On Mon, Dec 4, 2017 at 4:41 AM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> This commit enables I2S, digital and analog parts of audiocodec on
> Pine64 and SoPine boards.
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
> arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 16 ++++++++++++++++
> .../boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts | 16 ++++++++++++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> index 806442d3e846..369d9b749521 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
> @@ -64,6 +64,18 @@
> };
> };
>
> +&codec {
> + status = "okay";
> +};
> +
> +&codec_analog {
> + status = "okay";
> +};
> +
> +&dai {
> + status = "okay";
> +};
> +
> &ehci0 {
> status = "okay";
> };
> @@ -229,6 +241,10 @@
> regulator-name = "vcc-rtc";
> };
>
> +&sound {
> + status = "okay";
This is missing all the board level widgets and routing.
Yes I know that it works right now, but that's because simple-card
isn't a strict card, i.e. it doesn't force the need for actual inputs
and outputs at both ends of the DAPM graph.
Later on you'll find that missing the routing leaves the microphone
unusable, because the microphone bias is not turned on.
ChenYu
> +};
> +
> /* On Exp and Euler connectors */
> &uart0 {
> pinctrl-names = "default";
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> index 0eb2acedf8c3..9de5ddcc3656 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
> @@ -69,6 +69,18 @@
> };
> };
>
> +&codec {
> + status = "okay";
> +};
> +
> +&codec_analog {
> + status = "okay";
> +};
> +
> +&dai {
> + status = "okay";
> +};
> +
> &ehci0 {
> status = "okay";
> };
> @@ -133,6 +145,10 @@
> regulator-name = "vcc-wifi";
> };
>
> +&sound {
> + status = "okay";
> +};
> +
> &uart0 {
> pinctrl-names = "default";
> pinctrl-0 = <&uart0_pins_a>;
> --
> 2.15.0
>
^ permalink raw reply
* [linux-sunxi] [PATCH 0/2] clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module clocks
From: Chen-Yu Tsai @ 2017-12-05 3:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <25d0eff3-a0d9-fb49-8287-e65a4a102db7@arm.com>
On Tue, Dec 5, 2017 at 7:25 AM, Andr? Przywara <andre.przywara@arm.com> wrote:
> On 04/12/17 05:19, Chen-Yu Tsai wrote:
>> Hi,
>>
>> This is a small fix to get MMC performance up to proper speeds on the
>
> Maybe a small fix for a skilled developer, but a giant leap for all
> users ;-)
> MMC performance goes from: (4.15-rc1)
>
> SD: Timing buffered disk reads: 36 MB in 3.17 seconds = 11.35 MB/sec
> eMMC: Timing buffered disk reads: 66 MB in 3.03 seconds = 21.81 MB/sec
>
> to: (4.15-rc2 plus those two patches)
>
> SD: Timing buffered disk reads: 68 MB in 3.01 seconds = 22.61 MB/sec
> eMMC: Timing buffered disk reads: 132 MB in 3.01 seconds = 43.80 MB/sec
>
> So yes, factor of two ...
>
> Tested-by: Andre Przywara <andre.przywara@arm.com>
>
> Given the impact I wonder if this is a candidate for stable as well.
It could. But then again, nothing was broken. And it depends on the first
patch. I'm not sure stable would like that.
>
>> A64. According to the BSP kernel, the MMC module clocks have a /2 fixed
>> post-divider between the clock output and the MMC module, like what
>> we've seen with the "new MMC timing mode" on the A83T, but the A64 does
>> not have the mode switch.
>>
>> Sub-par performance was observed on the Banana Pi M64 eMMC. It only
>> reached half the read throughput of other Banana Pi boards, using a
>> standard sequential readout with a large block size. After these
>> patches, the performance is up to spec.
>>
>> The A64 can also do DDR transfer modes, but the clock delay config
>> registers in the MMC module are different from what we've seen so
>> far.
>
> But the BSP doesn't set those as well, does it? I mean to remember that
> they were all zero, expect for HS200/HS400?
What I meant was we should make it clear in the mmc driver that this
is another new configuration mechanism that is different from the
previous ones.
ChenYu
>
> Thanks!
> Andre.
>
>> One can just force enable DDR modes without tuning the delays,
>> and it does work. Proper support for this is left for another time.
>>
>>
>> ChenYu
>>
>> Chen-Yu Tsai (2):
>> clk: sunxi-ng: Support fixed post-dividers on MP style clocks
>> clk: sunxi-ng: sun50i: a64: Add 2x fixed post-divider to MMC module
>> clocks
>>
>> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 57 +++++++++++++++++++++++------------
>> drivers/clk/sunxi-ng/ccu_mp.c | 20 ++++++++++--
>> drivers/clk/sunxi-ng/ccu_mp.h | 24 +++++++++++++++
>> 3 files changed, 79 insertions(+), 22 deletions(-)
>>
>
^ permalink raw reply
* [linux-sunxi] [PATCH 1/2] clk: sunxi-ng: Support fixed post-dividers on MP style clocks
From: Chen-Yu Tsai @ 2017-12-05 3:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b961a1b6-f0e4-68e3-ab77-b0010dab63ff@arm.com>
On Tue, Dec 5, 2017 at 7:18 AM, Andr? Przywara <andre.przywara@arm.com> wrote:
> Hi Chen-Yu,
>
> On 04/12/17 05:19, Chen-Yu Tsai wrote:
>> On the A64, the MMC module clocks are fixed in the new timing mode,
>> i.e. they do not have a bit to select the mode. These clocks have
>> a 2x divider somewhere between the clock and the MMC module.
>>
>> To be consistent with other SoCs supporting the new timing mode,
>> we model the 2x divider as a fixed post-divider on the MMC module
>> clocks.
>>
>> To do this, we first add fixed post-divider to the MP style clocks,
>> which the MMC module clocks are.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>> drivers/clk/sunxi-ng/ccu_mp.c | 20 ++++++++++++++++++--
>> drivers/clk/sunxi-ng/ccu_mp.h | 24 ++++++++++++++++++++++++
>> 2 files changed, 42 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
>> index 688855e7dc8c..5d0af4051737 100644
>> --- a/drivers/clk/sunxi-ng/ccu_mp.c
>> +++ b/drivers/clk/sunxi-ng/ccu_mp.c
>> @@ -50,12 +50,19 @@ static unsigned long ccu_mp_round_rate(struct ccu_mux_internal *mux,
>> unsigned int max_m, max_p;
>> unsigned int m, p;
>>
>> + if (cmp->common.features & CCU_FEATURE_FIXED_POSTDIV)
>> + rate *= cmp->fixed_post_div;
>
> Can't you just initialise fixed_post_div to 1 normally and save the
> CCU_FEATURE_FIXED_POSTDIV?
I'll refer to Maxime about this. The feature flag was there from day
one. We only started to implement support for it later on. I'm not
sure if there was a reason to add them as feature flags, instead of
a field that defaults to something (0 even).
Otherwise it's a reasonable change. And we probably don't have to
do a wholesale change for the other clocks in one go. Incidentally
I have a A83T audio series that also adds post-dividers for another
clock type. I'll wait for a conclusion on this end before posting
it.
>
>> +
>> max_m = cmp->m.max ?: 1 << cmp->m.width;
>> max_p = cmp->p.max ?: 1 << ((1 << cmp->p.width) - 1);
>>
>> ccu_mp_find_best(*parent_rate, rate, max_m, max_p, &m, &p);
>> + rate = *parent_rate / p / m;
>> +
>> + if (cmp->common.features & CCU_FEATURE_FIXED_POSTDIV)
>> + rate /= cmp->fixed_post_div;
>>
>> - return *parent_rate / p / m;
>> + return rate;
>> }
>>
>> static void ccu_mp_disable(struct clk_hw *hw)
>> @@ -83,6 +90,7 @@ static unsigned long ccu_mp_recalc_rate(struct clk_hw *hw,
>> unsigned long parent_rate)
>> {
>> struct ccu_mp *cmp = hw_to_ccu_mp(hw);
>> + unsigned long rate;
>> unsigned int m, p;
>> u32 reg;
>>
>> @@ -101,7 +109,11 @@ static unsigned long ccu_mp_recalc_rate(struct clk_hw *hw,
>> p = reg >> cmp->p.shift;
>> p &= (1 << cmp->p.width) - 1;
>>
>> - return (parent_rate >> p) / m;
>> + rate = (parent_rate >> p) / m;
>> + if (cmp->common.features & CCU_FEATURE_FIXED_POSTDIV)
>> + rate /= cmp->fixed_post_div;
>> +
>> + return rate;
>> }
>>
>> static int ccu_mp_determine_rate(struct clk_hw *hw,
>> @@ -129,6 +141,10 @@ static int ccu_mp_set_rate(struct clk_hw *hw, unsigned long rate,
>> max_m = cmp->m.max ?: 1 << cmp->m.width;
>> max_p = cmp->p.max ?: 1 << ((1 << cmp->p.width) - 1);
>>
>> + /* Adjust target rate according to post-dividers */
>> + if (cmp->common.features & CCU_FEATURE_FIXED_POSTDIV)
>> + rate = rate * cmp->fixed_post_div;
>> +
>> ccu_mp_find_best(parent_rate, rate, max_m, max_p, &m, &p);
>>
>> spin_lock_irqsave(cmp->common.lock, flags);
>> diff --git a/drivers/clk/sunxi-ng/ccu_mp.h b/drivers/clk/sunxi-ng/ccu_mp.h
>> index aaef11d747ea..5107635e61de 100644
>> --- a/drivers/clk/sunxi-ng/ccu_mp.h
>> +++ b/drivers/clk/sunxi-ng/ccu_mp.h
>> @@ -33,9 +33,33 @@ struct ccu_mp {
>> struct ccu_div_internal m;
>> struct ccu_div_internal p;
>> struct ccu_mux_internal mux;
>> +
>> + unsigned int fixed_post_div;
>> +
>> struct ccu_common common;
>> };
>>
>> +#define SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(_struct, _name, _parents, _reg, \
>> + _mshift, _mwidth, \
>> + _pshift, _pwidth, \
>> + _muxshift, _muxwidth, \
>> + _gate, _postdiv, _flags) \
>> + struct ccu_mp _struct = { \
>> + .enable = _gate, \
>> + .m = _SUNXI_CCU_DIV(_mshift, _mwidth), \
>> + .p = _SUNXI_CCU_DIV(_pshift, _pwidth), \
>> + .mux = _SUNXI_CCU_MUX(_muxshift, _muxwidth), \
>> + .fixed_post_div = _postdiv, \
>> + .common = { \
>> + .reg = _reg, \
>> + .features = CCU_FEATURE_FIXED_POSTDIV, \
>> + .hw.init = CLK_HW_INIT_PARENTS(_name, \
>> + _parents, \
>> + &ccu_mp_ops, \
>> + _flags), \
>> + } \
>> + }
>> +
>
> This looks suspiciously like a copy of the macro below. What about you
> define the one below as a special case of this new one above?
Correct. But you can't unset the feature flag. So a copy is needed.
> Should be even more straightforward with defaulting postdiv to 1 and
> loosing the feature flags.
See above about the feature flag.
ChenYu
>
> Cheers,
> Andre.
>
>> #define SUNXI_CCU_MP_WITH_MUX_GATE(_struct, _name, _parents, _reg, \
>> _mshift, _mwidth, \
>> _pshift, _pwidth, \
>>
>
^ permalink raw reply
* [PATCH v4] usb: xhci: allow imod-interval to be configurable
From: Adam Wallis @ 2017-12-05 2:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512440100.17567.172.camel@mhfsdcap03>
On 12/4/2017 9:15 PM, Chunfeng Yun wrote:
> On Mon, 2017-12-04 at 09:27 -0500, Adam Wallis wrote:
>> The xHCI driver currently has the IMOD set to 160, which
>> translates to an IMOD interval of 40,000ns (160 * 250)ns
>>
>> Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
>> introduced a QUIRK for the MTK platform to adjust this interval to 20,
>> which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is
>> due to the fact that the MTK controller IMOD interval is 8 times
>> as much as defined in xHCI spec.
>>
>> Instead of adding more quirk bits for additional platforms, this patch
>> introduces the ability for vendors to set the IMOD_INTERVAL as is
>> optimal for their platform. By using device_property_read_u32() on
>> "imod-interval-ns", the IMOD INTERVAL can be specified in nano seconds.
>> If no interval is specified, the default of 40,000ns (IMOD=160) will be
>> used.
>>
>> No bounds checking has been implemented due to the fact that a vendor
>> may have violated the spec and would need to specify a value outside of
>> the max 8,000 IRQs/second limit specified in the xHCI spec.
>>
>> Tested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>> Signed-off-by: Adam Wallis <awallis@codeaurora.org>
>> ---
>> changes from v3:
>> * Changed imod-interval to imod-interval-ns [Rob Herring/Chunfeng]
>> * Changed "modulation" to "moderation" throughout patch [Mathias]
>> changes from v2:
>> * Added PCI default value [Mathias]
>> * Removed xhci-mtk.h from xhci-plat.c [Chunfeng Yun]
>> * Removed MTK quirk from xhci-plat and moved logic to xhci-mtk [Chunfeng]
>> * Updated bindings Documentation to use proper units [Rob Herring]
>> * Added imod-interval description and example to MTK binding documentation
>> changes from v1:
>> * Removed device_property_read_u32() per suggestion from greg k-h
>> * Used ER_IRQ_INTERVAL_MASK in place of (u16) cast
>>
>> Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
>> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
>> drivers/usb/host/xhci-mtk.c | 9 +++++++++
>> drivers/usb/host/xhci-pci.c | 3 +++
>> drivers/usb/host/xhci-plat.c | 5 +++++
>> drivers/usb/host/xhci.c | 7 ++-----
>> drivers/usb/host/xhci.h | 2 ++
>> 7 files changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>> index 3059596..9ff5602 100644
>> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>> @@ -46,6 +46,7 @@ Optional properties:
>> - pinctrl-names : a pinctrl state named "default" must be defined
>> - pinctrl-0 : pin control group
>> See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>> + - imod-interval-ns: default interrupt moderation interval is 5000ns
>>
>> Example:
>> usb30: usb at 11270000 {
>> @@ -66,6 +67,7 @@ usb30: usb at 11270000 {
>> usb3-lpm-capable;
>> mediatek,syscon-wakeup = <&pericfg>;
>> mediatek,wakeup-src = <1>;
>> + imod-interval-ns = <10000>;
>> };
>>
>> 2nd: dual-role mode with xHCI driver
>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> index ae6e484..969908d 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> @@ -29,6 +29,7 @@ Optional properties:
>> - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
>> - usb3-lpm-capable: determines if platform is USB3 LPM capable
>> - quirk-broken-port-ped: set if the controller has broken port disable mechanism
>> + - imod-interval-ns: default interrupt moderation interval is 5000ns
>>
>> Example:
>> usb at f0931000 {
>> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
>> index b62a1d2..1cb2a8b 100644
>> --- a/drivers/usb/host/xhci-mtk.c
>> +++ b/drivers/usb/host/xhci-mtk.c
>> @@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>>
>> xhci = hcd_to_xhci(hcd);
>> xhci->main_hcd = hcd;
>> +
>> + /*
>> + * imod_interval is the interrupt moderation value in nanoseconds.
>> + * The increment interval is 8 times as much as that defined in
>> + * the xHCI spec on MTK's controller.
>> + */
>> + xhci->imod_interval = 5000;
>> + device_property_read_u32(dev, "imod-interval-ns", &xhci->imod_interval);
>> +
>> xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
>> dev_name(dev), hcd);
>> if (!xhci->shared_hcd) {
>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>> index 7ef1274..4bcddd4 100644
>> --- a/drivers/usb/host/xhci-pci.c
>> +++ b/drivers/usb/host/xhci-pci.c
>> @@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
>> if (!xhci->sbrn)
>> pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, &xhci->sbrn);
>>
>> + /* imod_interval is the interrupt moderation value in nanoseconds. */
>> + xhci->imod_interval = 40000;
>> +
>> retval = xhci_gen_setup(hcd, xhci_pci_quirks);
>> if (retval)
>> return retval;
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 09f164f..6f03830 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -269,6 +269,11 @@ static int xhci_plat_probe(struct platform_device *pdev)
>> if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
>> xhci->quirks |= XHCI_BROKEN_PORT_PED;
>>
>> + /* imod_interval is the interrupt moderation value in nanoseconds. */
>> + xhci->imod_interval = 40000;
>> + device_property_read_u32(sysdev, "imod-interval-ns",
>> + &xhci->imod_interval);
>> +
>> hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
>> if (IS_ERR(hcd->usb_phy)) {
>> ret = PTR_ERR(hcd->usb_phy);
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index 2424d30..0b7755b 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
>> "// Set the interrupt modulation register");
> s/modulation/moderation
Mathias said there was no need to change the existing modulation strings - only
the ones that I had added.
>
>> temp = readl(&xhci->ir_set->irq_control);
>> temp &= ~ER_IRQ_INTERVAL_MASK;
>> - /*
>> - * the increment interval is 8 times as much as that defined
>> - * in xHCI spec on MTK's controller
>> - */
>> - temp |= (u32) ((xhci->quirks & XHCI_MTK_HOST) ? 20 : 160);
>> + temp |= (xhci->imod_interval / 250) & ER_IRQ_INTERVAL_MASK;
>> +
> No need a blank line
If this patch goes through another version, I will remove this line
>
>> writel(temp, &xhci->ir_set->irq_control);
>>
>> /* Set the HCD state before we enable the irqs */
>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>> index 99a014a..2a4177b 100644
>> --- a/drivers/usb/host/xhci.h
>> +++ b/drivers/usb/host/xhci.h
>> @@ -1717,6 +1717,8 @@ struct xhci_hcd {
>> u8 max_interrupters;
>> u8 max_ports;
>> u8 isoc_threshold;
>> + /* imod_interval in ns (I * 250ns) */
>> + u32 imod_interval;
>> int event_ring_max;
>> /* 4KB min, 128MB max */
>> int page_size;
>
> Thanks
>
>
--
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH v2 3/3] staging: vc04_services: Use __func__
From: Genki Sky @ 2017-12-05 2:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1512441073.git.sky@genki.is>
Signed-off-by: Genki Sky <sky@genki.is>
---
drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 5ddea4f54bf7..6c4d8b4c7cd9 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -618,8 +618,8 @@ static void buffer_to_host_cb(struct vchiq_mmal_instance *instance,
struct mmal_msg_context *msg_context;
u32 handle;
- pr_debug("buffer_to_host_cb: instance:%p msg:%p msg_len:%d\n",
- instance, msg, msg_len);
+ pr_debug("%s: instance:%p msg:%p msg_len:%d\n",
+ __func__, instance, msg, msg_len);
if (msg->u.buffer_from_host.drvbuf.magic == MMAL_MAGIC) {
handle = msg->u.buffer_from_host.drvbuf.client_context;
--
2.15.1
^ permalink raw reply related
* [PATCH v2 2/3] staging: vc04_services: Unsplit user-visible strings
From: Genki Sky @ 2017-12-05 2:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1512441073.git.sky@genki.is>
Signed-off-by: Genki Sky <sky@genki.is>
---
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 7 +++----
drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 3 +--
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 4ed3b449f97f..e26895dc052e 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -347,8 +347,7 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
timestamp = ktime_add_us(dev->capture.kernel_start_ts,
runtime_us);
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
- "Convert start time %llu and %llu "
- "with offset %llu to %llu\n",
+ "Convert start time %llu and %llu with offset %llu to %llu\n",
ktime_to_ns(dev->capture.kernel_start_ts),
dev->capture.vc_start_timestamp, pts,
ktime_to_ns(timestamp));
@@ -532,8 +531,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
vchiq_mmal_port_enable(dev->instance, dev->capture.port, buffer_cb);
if (ret) {
v4l2_err(&dev->v4l2_dev,
- "Failed to enable capture port - error %d. "
- "Disabling camera port again\n", ret);
+ "Failed to enable capture port - error %d. Disabling camera port again\n",
+ ret);
vchiq_mmal_port_disable(dev->instance,
dev->capture.camera_port);
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 6ea7fb0ea50e..5ddea4f54bf7 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -1360,8 +1360,7 @@ static int port_action_handle(struct vchiq_mmal_instance *instance,
ret = -rmsg->u.port_action_reply.status;
- pr_debug("%s:result:%d component:0x%x port:%d action:%s(%d)" \
- " connect component:0x%x connect port:%d\n",
+ pr_debug("%s:result:%d component:0x%x port:%d action:%s(%d) connect component:0x%x connect port:%d\n",
__func__,
ret, port->component->handle, port->handle,
port_action_type_names[action_type],
--
2.15.1
^ permalink raw reply related
* [PATCH v2 1/3] staging: vc04_services: Join multiline dereferences
From: Genki Sky @ 2017-12-05 2:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1512441073.git.sky@genki.is>
Signed-off-by: Genki Sky <sky@genki.is>
---
.../vc04_services/bcm2835-camera/bcm2835-camera.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index d8766b166675..4ed3b449f97f 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -328,11 +328,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
pr_debug("Grab another frame");
vchiq_mmal_port_parameter_set(
instance,
- dev->capture.
- camera_port,
+ dev->capture.camera_port,
MMAL_PARAMETER_CAPTURE,
- &dev->capture.
- frame_count,
+ &dev->capture.frame_count,
sizeof(dev->capture.frame_count));
}
} else {
@@ -368,11 +366,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
"Grab another frame as buffer has EOS");
vchiq_mmal_port_parameter_set(
instance,
- dev->capture.
- camera_port,
+ dev->capture.camera_port,
MMAL_PARAMETER_CAPTURE,
- &dev->capture.
- frame_count,
+ &dev->capture.frame_count,
sizeof(dev->capture.frame_count));
}
} else {
@@ -1194,8 +1190,8 @@ static int mmal_setup_components(struct bm2835_mmal_dev *dev,
port->current_buffer.size =
(f->fmt.pix.sizeimage <
(100 << 10))
- ? (100 << 10) : f->fmt.pix.
- sizeimage;
+ ? (100 << 10)
+ : f->fmt.pix.sizeimage;
}
v4l2_dbg(1, bcm2835_v4l2_debug,
&dev->v4l2_dev,
--
2.15.1
^ permalink raw reply related
* [PATCH v2 0/3] staging: vc04_services: Fix trivial style issues
From: Genki Sky @ 2017-12-05 2:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171204105235.2nhmalxsd4uslesi@mwanda>
Hi,
This is v2 of a patch series to fix some checkpatch.pl warnings in
drivers/staging/vc04_services/bcm2835-camera/*
Changes from v1 [1]:
- Split single patch into multiple patches
- Remove reference to eudyptula in commit message
[1]: https://lkml.kernel.org/r/<30cc7b48def470f1bec8d8d255044f4f220531ee.1512368114.git.sky@genki.is>
[ aside: this is for task-10 of http://eudyptula-challenge.org/ ]
Genki Sky (3):
staging: vc04_services: Join multiline dereferences
staging: vc04_services: Unsplit user-visible strings
staging: vc04_services: Use __func__
.../vc04_services/bcm2835-camera/bcm2835-camera.c | 23 +++++++++-------------
.../vc04_services/bcm2835-camera/mmal-vchiq.c | 7 +++----
2 files changed, 12 insertions(+), 18 deletions(-)
--
2.15.1
^ permalink raw reply
* [PATCH v3 2/3] clk: hisilicon: Add support for Hi3660 stub clocks
From: Leo Yan @ 2017-12-05 2:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1510910852-2175-3-git-send-email-xuyiping@hisilicon.com>
On Fri, Nov 17, 2017 at 05:27:31PM +0800, Xu YiPing wrote:
> From: Kaihua Zhong <zhongkaihua@huawei.com>
>
> Hi3660 has four stub clocks, which are big and LITTLE cluster clocks,
> GPU clock and DDR clock. These clocks ask MCU for frequency scaling
> by sending message through mailbox.
>
> This commit adds support for stub clocks, it requests the dedicated
> mailbox channel at initialization; then later uses this channel to send
> message to MCU to execute frequency scaling. The four stub clocks share
> the same mailbox channel, but every stub clock has its own command id so
> MCU can distinguish the requirement coming for which clock.
>
> A shared memory is used to present effective frequency value, so the
> clock driver uses I/O mapping for the memory and reads back rate value.
Hi Michael, Stephen,
Could you help review this patch?
> Reviewed-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: Kai Zhao <zhaokai1@hisilicon.com>
> Signed-off-by: Tao Wang <kevin.wangtao@hisilicon.com>
> Signed-off-by: Ruyi Wang <wangruyi@huawei.com>
> Signed-off-by: Kaihua Zhong <zhongkaihua@huawei.com>
> ---
> drivers/clk/hisilicon/Kconfig | 6 ++
> drivers/clk/hisilicon/Makefile | 1 +
> drivers/clk/hisilicon/clk-hi3660-stub.c | 186 ++++++++++++++++++++++++++++++++
> 3 files changed, 193 insertions(+)
> create mode 100644 drivers/clk/hisilicon/clk-hi3660-stub.c
>
> diff --git a/drivers/clk/hisilicon/Kconfig b/drivers/clk/hisilicon/Kconfig
> index 7098bfd..1bd4355 100644
> --- a/drivers/clk/hisilicon/Kconfig
> +++ b/drivers/clk/hisilicon/Kconfig
> @@ -49,3 +49,9 @@ config STUB_CLK_HI6220
> default ARCH_HISI
> help
> Build the Hisilicon Hi6220 stub clock driver.
> +
> +config STUB_CLK_HI3660
> + bool "Hi3660 Stub Clock Driver"
> + depends on COMMON_CLK_HI3660 && MAILBOX
> + help
> + Build the Hisilicon Hi3660 stub clock driver.
> diff --git a/drivers/clk/hisilicon/Makefile b/drivers/clk/hisilicon/Makefile
> index 1e4c3dd..0a5b499 100644
> --- a/drivers/clk/hisilicon/Makefile
> +++ b/drivers/clk/hisilicon/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_COMMON_CLK_HI3798CV200) += crg-hi3798cv200.o
> obj-$(CONFIG_COMMON_CLK_HI6220) += clk-hi6220.o
> obj-$(CONFIG_RESET_HISI) += reset.o
> obj-$(CONFIG_STUB_CLK_HI6220) += clk-hi6220-stub.o
> +obj-$(CONFIG_STUB_CLK_HI3660) += clk-hi3660-stub.o
> diff --git a/drivers/clk/hisilicon/clk-hi3660-stub.c b/drivers/clk/hisilicon/clk-hi3660-stub.c
> new file mode 100644
> index 0000000..607efa4
> --- /dev/null
> +++ b/drivers/clk/hisilicon/clk-hi3660-stub.c
> @@ -0,0 +1,186 @@
> +/*
> + * Hisilicon clock driver
> + *
> + * Copyright (c) 2013-2017 Hisilicon Limited.
> + * Copyright (c) 2017 Linaro Limited.
> + *
> + * Author: Kai Zhao <zhaokai1@hisilicon.com>
> + * Tao Wang <kevin.wangtao@hisilicon.com>
> + * Leo Yan <leo.yan@linaro.org>
> + *
> + * This program 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 program 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.
> + *
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <dt-bindings/clock/hi3660-clock.h>
> +
> +#define HI3660_STUB_CLOCK_DATA (0x70)
> +#define MHZ (1000 * 1000)
> +
> +#define DEFINE_CLK_STUB(_id, _cmd, _name) \
> + { \
> + .id = (_id), \
> + .cmd = (_cmd), \
> + .hw.init = &(struct clk_init_data) { \
> + .name = #_name, \
> + .ops = &hi3660_stub_clk_ops, \
> + .num_parents = 0, \
> + .flags = CLK_GET_RATE_NOCACHE, \
> + }, \
> + },
> +
> +#define to_stub_clk(_hw) container_of(_hw, struct hi3660_stub_clk, hw)
> +
> +struct hi3660_stub_clk_chan {
> + struct mbox_client cl;
> + struct mbox_chan *mbox;
> +};
> +
> +struct hi3660_stub_clk {
> + unsigned int id;
> + struct clk_hw hw;
> + unsigned int cmd;
> + unsigned int msg[8];
> + unsigned int rate;
> +};
> +
> +static void __iomem *freq_reg;
> +static struct hi3660_stub_clk_chan stub_clk_chan;
> +
> +static unsigned long hi3660_stub_clk_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct hi3660_stub_clk *stub_clk = to_stub_clk(hw);
> +
> + /*
> + * LPM3 writes back the CPU frequency in shared SRAM so read
> + * back the frequency.
> + */
> + stub_clk->rate = readl(freq_reg + (stub_clk->id << 2)) * MHZ;
> + return stub_clk->rate;
> +}
> +
> +static long hi3660_stub_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + /*
> + * LPM3 handles rate rounding so just return whatever
> + * rate is requested.
> + */
> + return rate;
> +}
> +
> +static int hi3660_stub_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct hi3660_stub_clk *stub_clk = to_stub_clk(hw);
> +
> + stub_clk->msg[0] = stub_clk->cmd;
> + stub_clk->msg[1] = rate / MHZ;
> +
> + dev_dbg(stub_clk_chan.cl.dev, "set rate msg[0]=0x%x msg[1]=0x%x\n",
> + stub_clk->msg[0], stub_clk->msg[1]);
> +
> + mbox_send_message(stub_clk_chan.mbox, stub_clk->msg);
> + mbox_client_txdone(stub_clk_chan.mbox, 0);
> +
> + stub_clk->rate = rate;
> + return 0;
> +}
> +
> +static const struct clk_ops hi3660_stub_clk_ops = {
> + .recalc_rate = hi3660_stub_clk_recalc_rate,
> + .round_rate = hi3660_stub_clk_round_rate,
> + .set_rate = hi3660_stub_clk_set_rate,
> +};
> +
> +static struct hi3660_stub_clk hi3660_stub_clks[HI3660_CLK_STUB_NUM] = {
> + DEFINE_CLK_STUB(HI3660_CLK_STUB_CLUSTER0, 0x0001030A, "cpu-cluster.0")
> + DEFINE_CLK_STUB(HI3660_CLK_STUB_CLUSTER1, 0x0002030A, "cpu-cluster.1")
> + DEFINE_CLK_STUB(HI3660_CLK_STUB_GPU, 0x0003030A, "clk-g3d")
> + DEFINE_CLK_STUB(HI3660_CLK_STUB_DDR, 0x00040309, "clk-ddrc")
> +};
> +
> +static struct clk_hw *hi3660_stub_clk_hw_get(struct of_phandle_args *clkspec,
> + void *data)
> +{
> + unsigned int idx = clkspec->args[0];
> +
> + if (idx > HI3660_CLK_STUB_NUM) {
> + pr_err("%s: invalid index %u\n", __func__, idx);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return &hi3660_stub_clks[idx].hw;
> +}
> +
> +static int hi3660_stub_clk_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + unsigned int i;
> + int ret;
> +
> + /* Use mailbox client without blocking */
> + stub_clk_chan.cl.dev = dev;
> + stub_clk_chan.cl.tx_done = NULL;
> + stub_clk_chan.cl.tx_block = false;
> + stub_clk_chan.cl.knows_txdone = false;
> +
> + /* Allocate mailbox channel */
> + stub_clk_chan.mbox = mbox_request_channel(&stub_clk_chan.cl, 0);
> + if (IS_ERR(stub_clk_chan.mbox))
> + return PTR_ERR(stub_clk_chan.mbox);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + freq_reg = devm_ioremap(dev, res->start, resource_size(res));
> + if (IS_ERR(freq_reg))
> + return -ENOMEM;
> +
> + freq_reg += HI3660_STUB_CLOCK_DATA;
> +
> + for (i = 0; i < HI3660_CLK_STUB_NUM; i++) {
> + ret = devm_clk_hw_register(&pdev->dev, &hi3660_stub_clks[i].hw);
> + if (ret)
> + return ret;
> + }
> +
> + ret = of_clk_add_hw_provider(pdev->dev.of_node, hi3660_stub_clk_hw_get,
> + hi3660_stub_clks);
> + return ret;
> +}
> +
> +static const struct of_device_id hi3660_stub_clk_of_match[] = {
> + { .compatible = "hisilicon,hi3660-stub-clk", },
> + {}
> +};
> +
> +static struct platform_driver hi3660_stub_clk_driver = {
> + .probe = hi3660_stub_clk_probe,
> + .driver = {
> + .name = "hi3660-stub-clk",
> + .of_match_table = hi3660_stub_clk_of_match,
> + },
> +};
> +
> +static int __init hi3660_stub_clk_init(void)
> +{
> + return platform_driver_register(&hi3660_stub_clk_driver);
> +}
> +subsys_initcall(hi3660_stub_clk_init);
> --
> 1.9.1
>
^ permalink raw reply
* [PATCH v3 2/3] mailbox: Add support for Hi3660 mailbox
From: Leo Yan @ 2017-12-05 2:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1510910672-1409-3-git-send-email-xuyiping@hisilicon.com>
On Fri, Nov 17, 2017 at 05:24:31PM +0800, Xu YiPing wrote:
> From: Kaihua Zhong <zhongkaihua@huawei.com>
>
> Hi3660 mailbox controller is used to send message within multiple
> processors, MCU, HIFI, etc. It supports 32 mailbox channels and every
> channel can only be used for single transferring direction. Once the
> channel is enabled, it needs to specify the destination interrupt and
> acknowledge interrupt, these two interrupt vectors are used to create
> the connection between the mailbox and interrupt controllers.
>
> The data transferring supports two modes, one is named as "automatic
> acknowledge" mode so after send message the kernel doesn't need to wait
> for acknowledge from remote and directly return; there have another mode
> is to rely on handling interrupt for acknowledge.
>
> This commit is for initial version driver, which only supports
> "automatic acknowledge" mode to support CPU clock, which is the only
> one consumer to use mailbox and has been verified. Later may enhance
> this driver for interrupt mode (e.g. for supporting HIFI).
Gentle ping ...
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: Ruyi Wang <wangruyi@huawei.com>
> Signed-off-by: Kaihua Zhong <zhongkaihua@huawei.com>
> ---
> drivers/mailbox/Kconfig | 8 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/hi3660-mailbox.c | 322 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 332 insertions(+)
> create mode 100644 drivers/mailbox/hi3660-mailbox.c
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index c5731e5..4b5d6e9 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -108,6 +108,14 @@ config TI_MESSAGE_MANAGER
> multiple processors within the SoC. Select this driver if your
> platform has support for the hardware block.
>
> +config HI3660_MBOX
> + tristate "Hi3660 Mailbox"
> + depends on ARCH_HISI && OF
> + help
> + An implementation of the hi3660 mailbox. It is used to send message
> + between application processors and other processors/MCU/DSP. Select
> + Y here if you want to use Hi3660 mailbox controller.
> +
> config HI6220_MBOX
> tristate "Hi6220 Mailbox"
> depends on ARCH_HISI
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index d54e412..7d1bd51 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o
>
> obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o
>
> +obj-$(CONFIG_HI3660_MBOX) += hi3660-mailbox.o
> +
> obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o
>
> obj-$(CONFIG_BCM_PDC_MBOX) += bcm-pdc-mailbox.o
> diff --git a/drivers/mailbox/hi3660-mailbox.c b/drivers/mailbox/hi3660-mailbox.c
> new file mode 100644
> index 0000000..ba80834
> --- /dev/null
> +++ b/drivers/mailbox/hi3660-mailbox.c
> @@ -0,0 +1,322 @@
> +/*
> + * Hisilicon's Hi3660 mailbox controller driver
> + *
> + * Copyright (c) 2017 Hisilicon Limited.
> + * Copyright (c) 2017 Linaro Limited.
> + *
> + * Author: Leo Yan <leo.yan@linaro.org>
> + *
> + * This program 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, version 2 of the License.
> + *
> + * This program 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.
> + *
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "mailbox.h"
> +
> +#define MBOX_CHAN_MAX 32
> +
> +#define MBOX_RX (0x0)
> +#define MBOX_TX (0x1)
> +
> +#define MBOX_BASE(mbox, ch) ((mbox)->base + ((ch) * 0x40))
> +#define MBOX_SRC_REG (0x00)
> +#define MBOX_DST_REG (0x04)
> +#define MBOX_DCLR_REG (0x08)
> +#define MBOX_DSTAT_REG (0x0c)
> +#define MBOX_MODE_REG (0x10)
> +#define MBOX_IMASK_REG (0x14)
> +#define MBOX_ICLR_REG (0x18)
> +#define MBOX_SEND_REG (0x1c)
> +#define MBOX_DATA_REG (0x20)
> +
> +#define MBOX_IPC_LOCK_REG (0xa00)
> +#define MBOX_IPC_UNLOCK (0x1acce551)
> +
> +#define MBOX_AUTOMATIC_ACK (1)
> +
> +#define MBOX_STATE_IDLE BIT(4)
> +#define MBOX_STATE_ACK BIT(7)
> +
> +#define MBOX_MSG_LEN 8
> +
> +/**
> + * Hi3660 mailbox channel device data
> + *
> + * A channel can be used for TX or RX, it can trigger remote
> + * processor interrupt to notify remote processor and can receive
> + * interrupt if has incoming message.
> + *
> + * @dst_irq: Interrupt vector for remote processor
> + * @ack_irq: Interrupt vector for local processor
> + */
> +struct hi3660_mbox_dev {
> + unsigned int dst_irq;
> + unsigned int ack_irq;
> +};
> +
> +/**
> + * Hi3660 mailbox controller data
> + *
> + * Mailbox controller includes 32 channels and can allocate
> + * channel for message transferring.
> + *
> + * @dev: Device to which it is attached
> + * @base: Base address of the register mapping region
> + * @chan: Representation of channels in mailbox controller
> + * @mdev: Representation of channel device data
> + * @controller: Representation of a communication channel controller
> + */
> +struct hi3660_mbox {
> + struct device *dev;
> + void __iomem *base;
> + struct mbox_chan chan[MBOX_CHAN_MAX];
> + struct hi3660_mbox_dev mdev[MBOX_CHAN_MAX];
> + struct mbox_controller controller;
> +};
> +
> +static inline struct hi3660_mbox *to_hi3660_mbox(struct mbox_controller *mbox)
> +{
> + return container_of(mbox, struct hi3660_mbox, controller);
> +}
> +
> +static int hi3660_mbox_check_state(struct mbox_chan *chan)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned long val;
> + unsigned int state, ret;
> +
> + /* Mailbox is idle so directly bail out */
> + state = readl_relaxed(base + MBOX_MODE_REG);
> + if (state & MBOX_STATE_IDLE)
> + return 0;
> +
> + /* Wait for acknowledge from remote */
> + ret = readx_poll_timeout_atomic(readl_relaxed, base + MBOX_MODE_REG,
> + val, (val & MBOX_STATE_ACK), 1000, 300000);
> + if (ret) {
> + dev_err(mbox->dev, "%s: timeout for receiving ack\n", __func__);
> + return ret;
> + }
> +
> + /* Ensure channel is released */
> + writel_relaxed(0xffffffff, base + MBOX_IMASK_REG);
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> +
> + return 0;
> +}
> +
> +static int hi3660_mbox_unlock(struct mbox_chan *chan)
> +{
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + unsigned int val, retry = 3;
> +
> + do {
> + writel_relaxed(MBOX_IPC_UNLOCK, mbox->base + MBOX_IPC_LOCK_REG);
> +
> + val = readl_relaxed(mbox->base + MBOX_IPC_LOCK_REG);
> + if (!val)
> + break;
> +
> + udelay(10);
> + } while (retry--);
> +
> + return (!val) ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_acquire_channel(struct mbox_chan *chan)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned int val, retry;
> +
> + for (retry = 10; retry; retry--) {
> + /* Check if channel has been acquired */
> + if (readl_relaxed(base + MBOX_MODE_REG) & MBOX_STATE_IDLE) {
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> + val = readl_relaxed(base + MBOX_SRC_REG);
> + if (val & BIT(mdev->ack_irq))
> + break;
> + }
> + }
> +
> + return retry ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_send(struct mbox_chan *chan, u32 *msg)
> +{
> + unsigned long ch = (unsigned long)chan->con_priv;
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> + void __iomem *base = MBOX_BASE(mbox, ch);
> + unsigned int i;
> +
> + /* Clear mask for destination interrupt */
> + writel_relaxed(~BIT(mdev->dst_irq), base + MBOX_IMASK_REG);
> +
> + /* Config destination for interrupt vector */
> + writel_relaxed(BIT(mdev->dst_irq), base + MBOX_DST_REG);
> +
> + /* Automatic acknowledge mode */
> + writel_relaxed(MBOX_AUTOMATIC_ACK, base + MBOX_MODE_REG);
> +
> + /* Fill message data */
> + for (i = 0; i < MBOX_MSG_LEN; i++)
> + writel_relaxed(msg[i], base + MBOX_DATA_REG + i * 4);
> +
> + /* Trigger data transferring */
> + writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SEND_REG);
> + return 0;
> +}
> +
> +static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)
> +{
> + struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> + int err;
> +
> + err = hi3660_mbox_check_state(chan);
> + if (err) {
> + dev_err(mbox->dev, "checking state failed\n");
> + return err;
> + }
> +
> + err = hi3660_mbox_unlock(chan);
> + if (err) {
> + dev_err(mbox->dev, "unlocking mailbox failed\n");
> + return err;
> + }
> +
> + err = hi3660_mbox_acquire_channel(chan);
> + if (err) {
> + dev_err(mbox->dev, "acquiring channel failed\n");
> + return err;
> + }
> +
> + return hi3660_mbox_send(chan, msg);
> +}
> +
> +static struct mbox_chan_ops hi3660_mbox_ops = {
> + .send_data = hi3660_mbox_send_data,
> +};
> +
> +static struct mbox_chan *hi3660_mbox_xlate(struct mbox_controller *controller,
> + const struct of_phandle_args *spec)
> +{
> + struct hi3660_mbox *mbox = to_hi3660_mbox(controller);
> + struct hi3660_mbox_dev *mdev;
> + unsigned int ch = spec->args[0];
> +
> + if (ch >= MBOX_CHAN_MAX) {
> + dev_err(mbox->dev, "Invalid channel idx %d\n", ch);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + mdev = &mbox->mdev[ch];
> + mdev->dst_irq = spec->args[1];
> + mdev->ack_irq = spec->args[2];
> +
> + return &mbox->chan[ch];
> +}
> +
> +static const struct of_device_id hi3660_mbox_of_match[] = {
> + { .compatible = "hisilicon,hi3660-mbox", },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, hi3660_mbox_of_match);
> +
> +static int hi3660_mbox_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct hi3660_mbox *mbox;
> + struct mbox_chan *chan;
> + struct resource *res;
> + unsigned long ch;
> + int err;
> +
> + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> + if (!mbox)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + mbox->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(mbox->base))
> + return PTR_ERR(mbox->base);
> +
> + mbox->dev = dev;
> + mbox->controller.dev = dev;
> + mbox->controller.chans = mbox->chan;
> + mbox->controller.num_chans = MBOX_CHAN_MAX;
> + mbox->controller.ops = &hi3660_mbox_ops;
> + mbox->controller.of_xlate = hi3660_mbox_xlate;
> +
> + /* Initialize mailbox channel data */
> + chan = mbox->chan;
> + for (ch = 0; ch < MBOX_CHAN_MAX; ch++)
> + chan[ch].con_priv = (void *)ch;
> +
> + err = mbox_controller_register(&mbox->controller);
> + if (err) {
> + dev_err(dev, "Failed to register mailbox %d\n", err);
> + return err;
> + }
> +
> + platform_set_drvdata(pdev, mbox);
> + dev_info(dev, "Mailbox enabled\n");
> + return 0;
> +}
> +
> +static int hi3660_mbox_remove(struct platform_device *pdev)
> +{
> + struct hi3660_mbox *mbox = platform_get_drvdata(pdev);
> +
> + mbox_controller_unregister(&mbox->controller);
> + return 0;
> +}
> +
> +static struct platform_driver hi3660_mbox_driver = {
> + .probe = hi3660_mbox_probe,
> + .remove = hi3660_mbox_remove,
> + .driver = {
> + .name = "hi3660-mbox",
> + .of_match_table = hi3660_mbox_of_match,
> + },
> +};
> +
> +static int __init hi3660_mbox_init(void)
> +{
> + return platform_driver_register(&hi3660_mbox_driver);
> +}
> +core_initcall(hi3660_mbox_init);
> +
> +static void __exit hi3660_mbox_exit(void)
> +{
> + platform_driver_unregister(&hi3660_mbox_driver);
> +}
> +module_exit(hi3660_mbox_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Hisilicon Hi3660 Mailbox Controller");
> +MODULE_AUTHOR("Leo Yan <leo.yan@linaro.org>");
> --
> 1.9.1
>
^ permalink raw reply
* [PATCH v4] usb: xhci: allow imod-interval to be configurable
From: Chunfeng Yun @ 2017-12-05 2:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512397671-28733-1-git-send-email-awallis@codeaurora.org>
On Mon, 2017-12-04 at 09:27 -0500, Adam Wallis wrote:
> The xHCI driver currently has the IMOD set to 160, which
> translates to an IMOD interval of 40,000ns (160 * 250)ns
>
> Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
> introduced a QUIRK for the MTK platform to adjust this interval to 20,
> which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is
> due to the fact that the MTK controller IMOD interval is 8 times
> as much as defined in xHCI spec.
>
> Instead of adding more quirk bits for additional platforms, this patch
> introduces the ability for vendors to set the IMOD_INTERVAL as is
> optimal for their platform. By using device_property_read_u32() on
> "imod-interval-ns", the IMOD INTERVAL can be specified in nano seconds.
> If no interval is specified, the default of 40,000ns (IMOD=160) will be
> used.
>
> No bounds checking has been implemented due to the fact that a vendor
> may have violated the spec and would need to specify a value outside of
> the max 8,000 IRQs/second limit specified in the xHCI spec.
>
> Tested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Signed-off-by: Adam Wallis <awallis@codeaurora.org>
> ---
> changes from v3:
> * Changed imod-interval to imod-interval-ns [Rob Herring/Chunfeng]
> * Changed "modulation" to "moderation" throughout patch [Mathias]
> changes from v2:
> * Added PCI default value [Mathias]
> * Removed xhci-mtk.h from xhci-plat.c [Chunfeng Yun]
> * Removed MTK quirk from xhci-plat and moved logic to xhci-mtk [Chunfeng]
> * Updated bindings Documentation to use proper units [Rob Herring]
> * Added imod-interval description and example to MTK binding documentation
> changes from v1:
> * Removed device_property_read_u32() per suggestion from greg k-h
> * Used ER_IRQ_INTERVAL_MASK in place of (u16) cast
>
> Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
> drivers/usb/host/xhci-mtk.c | 9 +++++++++
> drivers/usb/host/xhci-pci.c | 3 +++
> drivers/usb/host/xhci-plat.c | 5 +++++
> drivers/usb/host/xhci.c | 7 ++-----
> drivers/usb/host/xhci.h | 2 ++
> 7 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> index 3059596..9ff5602 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> @@ -46,6 +46,7 @@ Optional properties:
> - pinctrl-names : a pinctrl state named "default" must be defined
> - pinctrl-0 : pin control group
> See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> + - imod-interval-ns: default interrupt moderation interval is 5000ns
>
> Example:
> usb30: usb at 11270000 {
> @@ -66,6 +67,7 @@ usb30: usb at 11270000 {
> usb3-lpm-capable;
> mediatek,syscon-wakeup = <&pericfg>;
> mediatek,wakeup-src = <1>;
> + imod-interval-ns = <10000>;
> };
>
> 2nd: dual-role mode with xHCI driver
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index ae6e484..969908d 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -29,6 +29,7 @@ Optional properties:
> - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
> - usb3-lpm-capable: determines if platform is USB3 LPM capable
> - quirk-broken-port-ped: set if the controller has broken port disable mechanism
> + - imod-interval-ns: default interrupt moderation interval is 5000ns
>
> Example:
> usb at f0931000 {
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index b62a1d2..1cb2a8b 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>
> xhci = hcd_to_xhci(hcd);
> xhci->main_hcd = hcd;
> +
> + /*
> + * imod_interval is the interrupt moderation value in nanoseconds.
> + * The increment interval is 8 times as much as that defined in
> + * the xHCI spec on MTK's controller.
> + */
> + xhci->imod_interval = 5000;
> + device_property_read_u32(dev, "imod-interval-ns", &xhci->imod_interval);
> +
> xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
> dev_name(dev), hcd);
> if (!xhci->shared_hcd) {
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 7ef1274..4bcddd4 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
> if (!xhci->sbrn)
> pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, &xhci->sbrn);
>
> + /* imod_interval is the interrupt moderation value in nanoseconds. */
> + xhci->imod_interval = 40000;
> +
> retval = xhci_gen_setup(hcd, xhci_pci_quirks);
> if (retval)
> return retval;
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 09f164f..6f03830 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -269,6 +269,11 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
> xhci->quirks |= XHCI_BROKEN_PORT_PED;
>
> + /* imod_interval is the interrupt moderation value in nanoseconds. */
> + xhci->imod_interval = 40000;
> + device_property_read_u32(sysdev, "imod-interval-ns",
> + &xhci->imod_interval);
> +
> hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
> if (IS_ERR(hcd->usb_phy)) {
> ret = PTR_ERR(hcd->usb_phy);
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 2424d30..0b7755b 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
> "// Set the interrupt modulation register");
s/modulation/moderation
> temp = readl(&xhci->ir_set->irq_control);
> temp &= ~ER_IRQ_INTERVAL_MASK;
> - /*
> - * the increment interval is 8 times as much as that defined
> - * in xHCI spec on MTK's controller
> - */
> - temp |= (u32) ((xhci->quirks & XHCI_MTK_HOST) ? 20 : 160);
> + temp |= (xhci->imod_interval / 250) & ER_IRQ_INTERVAL_MASK;
> +
No need a blank line
> writel(temp, &xhci->ir_set->irq_control);
>
> /* Set the HCD state before we enable the irqs */
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 99a014a..2a4177b 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1717,6 +1717,8 @@ struct xhci_hcd {
> u8 max_interrupters;
> u8 max_ports;
> u8 isoc_threshold;
> + /* imod_interval in ns (I * 250ns) */
> + u32 imod_interval;
> int event_ring_max;
> /* 4KB min, 128MB max */
> int page_size;
Thanks
^ permalink raw reply
* [PATCH v9 3/5] perf utils: use pmu->is_uncore to detect PMU UNCORE devices
From: Jin, Yao @ 2017-12-05 2:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171016183222.25750-4-ganapatrao.kulkarni@cavium.com>
Hi Kulkarni, Arnaldo,
This patch has been merged in perf/core branch today.
But I see a regression issue when I run the 'perf stat'.
With bisect checking, I locate to this patch.
commit ad8737a08973f5dca632bdd63cf2abc99670e540
Author: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
Date: Tue Oct 17 00:02:20 2017 +0530
perf pmu: Use pmu->is_uncore to detect UNCORE devices
For example (on Intel skylake desktop),
1. The correct output should be (without this patch):
root at skl:/tmp# perf stat --per-thread -p 1754 -M CPI,IPC
^C
Performance counter stats for process id '1754':
vmstat-1754 1,882,798 inst_retired.any
# 0.8 CPI
vmstat-1754 1,589,720 cycles
vmstat-1754 1,882,798 inst_retired.any
# 1.2 IPC
vmstat-1754 1,589,720 cpu_clk_unhalted.thread
2.647443167 seconds time elapsed
2. With this patch, the output will be:
root at skl:/tmp# perf stat --per-thread -p 1754 -M CPI,IPC
^C
Performance counter stats for process id '1754':
vmstat-1754 1,945,589 inst_retired.any
vmstat-1754 <not supported> inst_retired.any
vmstat-1754 1,609,892 cycles
vmstat-1754 1,945,589 inst_retired.any
vmstat-1754 <not supported> inst_retired.any
vmstat-1754 1,609,892 cpu_clk_unhalted.thread
vmstat-1754 <not supported> cpu_clk_unhalted.thread
3.051274166 seconds time elapsed
Could you please help to take a look?
Thanks
Jin Yao
On 10/17/2017 2:32 AM, Ganapatrao Kulkarni wrote:
> PMU CORE devices are identified using sysfs filename cpu, however
> on some platforms(like arm/arm64), PMU CORE sysfs name is not cpu.
> Hence cpu cannot be used to differentiate PMU CORE/UNCORE devices.
>
> commit:
> 66ec1191 ("perf pmu: Unbreak perf record for arm/arm64 with events with explicit PMU")
>
> has introduced pmu->is_uncore, which is set to PMU UNCORE devices only.
> Adding changes to use pmu->is_uncore to identify UNCORE devices.
>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Tested-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
> tools/perf/util/pmu.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 8b17db5..9110718 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -603,7 +603,6 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
> */
> i = 0;
> while (1) {
> - const char *pname;
>
> pe = &map->table[i++];
> if (!pe->name) {
> @@ -612,9 +611,13 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
> break;
> }
>
> - pname = pe->pmu ? pe->pmu : "cpu";
> - if (strncmp(pname, name, strlen(pname)))
> - continue;
> + if (pmu->is_uncore) {
> + /* check for uncore devices */
> + if (pe->pmu == NULL)
> + continue;
> + if (strncmp(pe->pmu, name, strlen(pe->pmu)))
> + continue;
> + }
>
> /* need type casts to override 'const' */
> __perf_pmu__new_alias(head, NULL, (char *)pe->name,
>
^ permalink raw reply
* [PATCH 05/45] drivers: hwtracing: remove duplicate includes
From: Pravin Shedge @ 2017-12-05 2:00 UTC (permalink / raw)
To: linux-arm-kernel
These duplicate includes have been found with scripts/checkincludes.pl but
they have been removed manually to avoid removing false positives.
Signed-off-by: Pravin Shedge <pravin.shedge4linux@gmail.com>
---
drivers/hwtracing/coresight/coresight-etb10.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index e03e589..580cd38 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -33,7 +33,6 @@
#include <linux/mm.h>
#include <linux/perf_event.h>
-#include <asm/local.h>
#include "coresight-priv.h"
--
2.7.4
^ permalink raw reply related
* [PATCH 03/45] drivers: gpio: remove duplicate includes
From: Pravin Shedge @ 2017-12-05 1:58 UTC (permalink / raw)
To: linux-arm-kernel
These duplicate includes have been found with scripts/checkincludes.pl but
they have been removed manually to avoid removing false positives.
Signed-off-by: Pravin Shedge <pravin.shedge4linux@gmail.com>
---
drivers/gpio/gpio-brcmstb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index 545d43a..f40ec39 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -11,7 +11,6 @@
* GNU General Public License for more details.
*/
-#include <linux/bitops.h>
#include <linux/gpio/driver.h>
#include <linux/of_device.h>
#include <linux/of_irq.h>
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 2/2] net: ethernet: socionext: add AVE ethernet driver
From: Kunihiko Hayashi @ 2017-12-05 1:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171201134900.GE22599@lunn.ch>
Hi Andrew,
On Fri, 1 Dec 2017 14:49:00 +0100 Andrew Lunn <andrew@lunn.ch> wrote:
> On Fri, Dec 01, 2017 at 10:03:50AM +0900, Kunihiko Hayashi wrote:
> > The UniPhier platform from Socionext provides the AVE ethernet
> > controller that includes MAC and MDIO bus supporting RGMII/RMII
> > modes. The controller is named AVE.
> >
> > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> > Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>
> Hi Kunihiko
>
> I reviewed the PHY and MDIO code. It all looks good. I cannot say much
> about the rest of it though.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Thanks! I'll remove warning from kbuild test, and repost the series for
reviewing other part.
---
Best Regards,
Kunihiko Hayashi
^ permalink raw reply
* [PATCH 02/10] net: bcmgenet: free netdev on of_match_node() error
From: Doug Berger @ 2017-12-05 1:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512242782-7134-3-git-send-email-arvind.yadav.cs@gmail.com>
On 12/02/2017 11:26 AM, Arvind Yadav wrote:
> The change is to call free_netdev(), If of_match_node() will fail.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index e2f1268..e0a8f79 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3363,8 +3363,10 @@ static int bcmgenet_probe(struct platform_device *pdev)
>
> if (dn) {
> of_id = of_match_node(bcmgenet_match, dn);
> - if (!of_id)
> - return -EINVAL;
> + if (!of_id) {
> + err = -EINVAL;
> + goto err;
> + }
> }
>
> priv = netdev_priv(dev);
>
I agree with the fix if you want to resubmit separate from this series
and please include a "fixes" tag.
Thanks,
Doug
^ permalink raw reply
* [PATCH 1/7 v2] net: bcmgenet: Fix platform_get_irq's error checking
From: Doug Berger @ 2017-12-05 1:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512409703-20881-2-git-send-email-arvind.yadav.cs@gmail.com>
On 12/04/2017 09:48 AM, Arvind Yadav wrote:
> The platform_get_irq() function returns negative number if an error occurs,
> Zero if No irq is found and positive number if irq gets successful.
> platform_get_irq() error checking only for zero is not correct.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> changes in v2:
> commit message was not correct.
>
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 24b4f4c..e2f1268 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3371,7 +3371,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
> priv->irq0 = platform_get_irq(pdev, 0);
> priv->irq1 = platform_get_irq(pdev, 1);
> priv->wol_irq = platform_get_irq(pdev, 2);
> - if (!priv->irq0 || !priv->irq1) {
> + if (priv->irq0 <= 0 || priv->irq1 <= 0 || priv->wol_irq <= 0) {
> dev_err(&pdev->dev, "can't find IRQs\n");
> err = -EINVAL;
> goto err;
>
The absence of a Wake-on-LAN interrupt (wol_irq <= 0) is not a terminal
error for the driver so it should not be included in this check.
The error checking for irq0 and irq1 is appropriate to add, but it
sounds like David Miller is proposing changing platform_get_irq() so
I'll let that dust settle before saying whether <= or < is appropriate.
Thanks,
Doug
^ permalink raw reply
* [PATCH 4/8] ASoC: uniphier: add support for UniPhier EVEA codec
From: Masahiro Yamada @ 2017-12-05 0:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171204182027.2k5uw5og65h6ac7w@sirena.org.uk>
+CC Greg-KH
+CC Thomas Gleixner
2017-12-05 3:20 GMT+09:00 Mark Brown <broonie@kernel.org>:
> On Wed, Nov 22, 2017 at 08:43:17PM +0900, Katsuhiro Suzuki wrote:
>
>> +++ b/sound/soc/uniphier/evea.c
>> @@ -0,0 +1,567 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Socionext UniPhier EVEA ADC/DAC codec driver.
>> + *
>> + * Copyright (c) 2016-2017 Socionext Inc.
>
> Make the entire comment a C++ comment, don't mix and match like this -
> it's ugly. Otherwise this looks good so I'll apply it, please send a
> followup patch fixing this (and I guess the same thing will apply to
> other files).
Indeed ugly,
but I think this is intentional to make the SPDX line stand out.
Linus suggested this as far as I understood from the following:
https://patchwork.kernel.org/patch/10016201/
If you use C++ comment style for the entire block,
it will not stand out.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* [PATCH v2 00/18] arm64: Unmap the kernel whilst running in userspace (KAISER)
From: Laura Abbott @ 2017-12-04 23:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1512059986-21325-1-git-send-email-will.deacon@arm.com>
On 11/30/2017 08:39 AM, Will Deacon wrote:
> Hi again,
>
> This is version two of the patches previously posted here:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-November/542751.html
>
> Changes since v1 include:
>
> * Based on v4.15-rc1
> * Trampoline moved into FIXMAP area
> * Explicit static key replaced by cpu cap
> * Disable SPE for userspace profiling if kernel unmapped at EL0
> * Changed polarity of cpu feature to match config option
> * Changed command-line option so we can force on in future if necessary
> * Changed Falkor workaround to invalidate different page within 2MB region
> * Reworked alternative sequences in entry.S, since the NOP slides with
> kaiser=off were measurable
>
> I experimented with leaving the vbar set to point at the kaiser vectors,
> but I couldn't measure any performance improvement from that and it made
> the code slightly more complicated, so I've left it as-is.
>
> Patches based on 4.15-rc1 and also pushed here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git kaiser
>
> Feedback welcome, particularly on a better name for the command-line option.
>
I ran this with one of the LTP mmap tests over the weekend. The mmap
test completed successfully but later the machine was spewing I/O
errors. I think this is because of the hardware and not the patches
so I'm running again for good measure.
> Will
>
> --->8
>
> Will Deacon (18):
> arm64: mm: Use non-global mappings for kernel space
> arm64: mm: Temporarily disable ARM64_SW_TTBR0_PAN
> arm64: mm: Move ASID from TTBR0 to TTBR1
> arm64: mm: Remove pre_ttbr0_update_workaround for Falkor erratum
> #E1003
> arm64: mm: Rename post_ttbr0_update_workaround
> arm64: mm: Fix and re-enable ARM64_SW_TTBR0_PAN
> arm64: mm: Allocate ASIDs in pairs
> arm64: mm: Add arm64_kernel_unmapped_at_el0 helper
> arm64: mm: Invalidate both kernel and user ASIDs when performing TLBI
> arm64: entry: Add exception trampoline page for exceptions from EL0
> arm64: mm: Map entry trampoline into trampoline and kernel page tables
> arm64: entry: Explicitly pass exception level to kernel_ventry macro
> arm64: entry: Hook up entry trampoline to exception vectors
> arm64: erratum: Work around Falkor erratum #E1003 in trampoline code
> arm64: tls: Avoid unconditional zeroing of tpidrro_el0 for native
> tasks
> arm64: entry: Add fake CPU feature for unmapping the kernel at EL0
> arm64: Kconfig: Add CONFIG_UNMAP_KERNEL_AT_EL0
> perf: arm_spe: Disallow userspace profiling when
> arm_kernel_unmapped_at_el0()
>
> arch/arm64/Kconfig | 30 +++--
> arch/arm64/include/asm/asm-uaccess.h | 25 +++--
> arch/arm64/include/asm/assembler.h | 27 +----
> arch/arm64/include/asm/cpucaps.h | 3 +-
> arch/arm64/include/asm/fixmap.h | 4 +
> arch/arm64/include/asm/kernel-pgtable.h | 12 +-
> arch/arm64/include/asm/mmu.h | 10 ++
> arch/arm64/include/asm/mmu_context.h | 9 +-
> arch/arm64/include/asm/pgtable-hwdef.h | 1 +
> arch/arm64/include/asm/pgtable-prot.h | 21 +++-
> arch/arm64/include/asm/pgtable.h | 1 +
> arch/arm64/include/asm/proc-fns.h | 6 -
> arch/arm64/include/asm/tlbflush.h | 16 ++-
> arch/arm64/include/asm/uaccess.h | 21 +++-
> arch/arm64/kernel/asm-offsets.c | 6 +-
> arch/arm64/kernel/cpufeature.c | 41 +++++++
> arch/arm64/kernel/entry.S | 190 +++++++++++++++++++++++++++-----
> arch/arm64/kernel/process.c | 12 +-
> arch/arm64/kernel/vmlinux.lds.S | 17 +++
> arch/arm64/lib/clear_user.S | 2 +-
> arch/arm64/lib/copy_from_user.S | 2 +-
> arch/arm64/lib/copy_in_user.S | 2 +-
> arch/arm64/lib/copy_to_user.S | 2 +-
> arch/arm64/mm/cache.S | 2 +-
> arch/arm64/mm/context.c | 36 +++---
> arch/arm64/mm/mmu.c | 23 ++++
> arch/arm64/mm/proc.S | 12 +-
> arch/arm64/xen/hypercall.S | 2 +-
> drivers/perf/arm_spe_pmu.c | 7 ++
> 29 files changed, 407 insertions(+), 135 deletions(-)
>
^ 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