* [PATCH] arm64: avoid increasing DMA masks above what hardware supports
From: Robin Murphy @ 2017-01-10 17:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484056844-9567-1-git-send-email-nikita.yoush@cogentembedded.com>
On 10/01/17 14:00, Nikita Yushchenko wrote:
> There are cases when device supports wide DMA addresses wider than
> device's connection supports.
>
> In this case driver sets DMA mask based on knowledge of device
> capabilities. That must succeed to allow drivers to initialize.
>
> However, swiotlb or iommu still need knowledge about actual device
> capabilities. To avoid breakage, actual mask must not be set wider than
> device connection allows.
>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Robin Murphy <robin.murphy@arm.com>
> CC: Will Deacon <will.deacon@arm.com>
> ---
> arch/arm64/Kconfig | 3 +++
> arch/arm64/include/asm/device.h | 1 +
> arch/arm64/include/asm/dma-mapping.h | 3 +++
> arch/arm64/mm/dma-mapping.c | 43 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 50 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1117421..afb2c08 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -216,6 +216,9 @@ config NEED_DMA_MAP_STATE
> config NEED_SG_DMA_LENGTH
> def_bool y
>
> +config ARCH_HAS_DMA_SET_COHERENT_MASK
> + def_bool y
> +
> config SMP
> def_bool y
>
> diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
> index 243ef25..a57e7bb 100644
> --- a/arch/arm64/include/asm/device.h
> +++ b/arch/arm64/include/asm/device.h
> @@ -22,6 +22,7 @@ struct dev_archdata {
> void *iommu; /* private IOMMU data */
> #endif
> bool dma_coherent;
> + u64 parent_dma_mask;
> };
>
> struct pdev_archdata {
> diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
> index ccea82c..eab36d2 100644
> --- a/arch/arm64/include/asm/dma-mapping.h
> +++ b/arch/arm64/include/asm/dma-mapping.h
> @@ -51,6 +51,9 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> const struct iommu_ops *iommu, bool coherent);
> #define arch_setup_dma_ops arch_setup_dma_ops
>
> +#define HAVE_ARCH_DMA_SET_MASK 1
> +extern int dma_set_mask(struct device *dev, u64 dma_mask);
> +
> #ifdef CONFIG_IOMMU_DMA
> void arch_teardown_dma_ops(struct device *dev);
> #define arch_teardown_dma_ops arch_teardown_dma_ops
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index e040827..7b1bb87 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -203,6 +203,37 @@ static void __dma_free(struct device *dev, size_t size,
> __dma_free_coherent(dev, size, swiotlb_addr, dma_handle, attrs);
> }
>
> +int dma_set_mask(struct device *dev, u64 dma_mask)
> +{
> + const struct dma_map_ops *ops = get_dma_ops(dev);
> +
> + if (mask > dev->archdata.parent_dma_mask)
> + mask = dev->archdata.parent_dma_mask;
> +
> + if (ops->set_dma_mask)
> + return ops->set_dma_mask(dev, mask);
> +
> + if (!dev->dma_mask || !dma_supported(dev, mask))
> + return -EIO;
> +
> + *dev->dma_mask = mask;
> + return 0;
> +}
> +EXPORT_SYMBOL(dma_set_mask);
> +
> +int dma_set_coherent_mask(struct device *dev, u64 mask)
> +{
> + if (mask > dev->archdata.parent_dma_mask)
> + mask = dev->archdata.parent_dma_mask;
> +
> + if (!dma_supported(dev, mask))
> + return -EIO;
> +
> + dev->coherent_dma_mask = mask;
> + return 0;
> +}
> +EXPORT_SYMBOL(dma_set_coherent_mask);
> +
> static dma_addr_t __swiotlb_map_page(struct device *dev, struct page *page,
> unsigned long offset, size_t size,
> enum dma_data_direction dir,
> @@ -958,6 +989,18 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> if (!dev->archdata.dma_ops)
> dev->archdata.dma_ops = &swiotlb_dma_ops;
>
> + /*
> + * we don't yet support buses that have a non-zero mapping.
> + * Let's hope we won't need it
> + */
> + WARN_ON(dma_base != 0);
I believe we now accomodate the bus remap bits on BCM2837 as a DMA
offset, so unfortunately I think this is no longer true.
> + /*
> + * Whatever the parent bus can set. A device must not set
> + * a DMA mask larger than this.
> + */
> + dev->archdata.parent_dma_mask = size - 1;
This will effectively constrain *all* DMA masks to be 32-bit, since for
99% of devices we're going to see a size derived from the default mask
passed in here. I worry that that's liable to lead to performance and
stability regressions (now that the block layer can apparently generate
sufficient readahead to ovflow a typical SWIOTLB bounce buffer[1]).
Whilst DT users would be able to mitigate that by putting explicit
"dma-ranges" properties on every device node, it's less clear what we'd
do for ACPI.
I reckon the easiest way forward would be to pass in some flag to
arch_setup_dma_ops to indicate whether it's an explicitly-configured
range or not - then simply initialising parent_dma_mask to ~0 for the
default case *should* keep things working as before.
Robin.
[1]:https://www.mail-archive.com/virtualization at lists.linux-foundation.org/msg26532.html
> +
> dev->archdata.dma_coherent = coherent;
> __iommu_setup_dma_ops(dev, dma_base, size, iommu);
> }
>
^ permalink raw reply
* [PATCH v4 0/9] arm64: Expose CPUID registers via emulation
From: Will Deacon @ 2017-01-10 17:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1483982912-27215-1-git-send-email-suzuki.poulose@arm.com>
On Mon, Jan 09, 2017 at 05:28:23PM +0000, Suzuki K Poulose wrote:
> This series adds a new ABI to expose the CPU feature registers
> to the user space via emulation of MRS instruction. The system exposes
> only a limited set of feature values (See the documentation patch)
> from the cpufeature infrastructure. The feature bits that are not
> exposed are set to the 'safe value' which implies 'not supported'.
>
> Apart from the selected feature registers, we expose MIDR_EL1 (Main
> ID Register). The user should be aware that, reading MIDR_EL1 can be
> tricky on a heterogeneous system (just like getcpu()). We export the
> value of the current CPU where 'MRS' is executed.
>
> Applies on v4.10-rc3.
Thanks, I'll queue this for 4.11.
Will
^ permalink raw reply
* [PATCH v12 0/7] Enable PMUs in ACPI systems
From: Jeremy Linton @ 2017-01-10 17:17 UTC (permalink / raw)
To: linux-arm-kernel
This patch expands and reworks the patches published by Mark Salter
in order to clean up a few of the previous review comments, as well as
add support for newer CPUs and big/little configurations.
v12:
- Rebase to 4.10
- Removed lock in the hotplug path.
v11:
- Add is_smp() check to read_specific_cpuid() for arch/arm. Update
c_show() and various routines in arm_pmu_acpi() to use the macro.
- Moved the duplicate "generic" pmu detection code into its own
patch and hoist it into arm_pmu_device_probe() so it works
for DT based systems as well.
v10:
- Rebase to 4.9
- Rework the arm_perf_start_cpu changes to support the 4.9 hotplug
changes.
- Remove the call to acpi_register_gsi() from the cpu online code path.
Instead the GSI's are registered during the initcall. This changes
the error handling a bit because we now try to clean up the
previously registered GSIs in a couple important places. This
was also a result of the rebase.
- Dropped the MIDR partnumber usage, its no longer necessary to
differentiate by only the partnum, so this helps to clarify the code
a bit.
- Shuffle some code around and rename a few variables.
- Added a few comments to hopefully clarify some questions people have
previously had about unused MADT entries, skipping processing cores
with MIDR=0, etc.
v9:
- Add/cleanup an additional hotplug patch I've had sitting around. This
patch brings the ACPI PMU mostly on par with the DT functionality with
respect to having CPUs offline during boot. This should help clarify
some of the code structuring.
- Cleanup the list of PMU types early if we fail to allocate memory for an
additional pmu type.
v8:
- Rebase to 4.8rc4
- Assorted minor comment/hunk placement/etc tweaks per Punit Agrawal
v7:
- Rebase to 4.8rc3
- Remove cpu affinity sysfs entry. While providing a CPU mask for
ARMv8 PMU's is really helpful in big/little environments, reworking
the PMU code to support the cpumask attribute for !arm64 PMUs is out
of the scope of this patch set.
- Fix CPU miscount problem where an alloc failure followed by successfully
allocating the structure can result in under counting the CPUs associated
with the PMU. This bug was created in v6 with the conversion to a linked
list.
- Remove initial platform device creation code by Mark Salter, and re-squash
multiple platform device creation code together with helper routines.
Other minor tweakage.
Jeremy Linton (6):
arm64: Rename the common MADT parse routine
arm: arm64: Add routine to determine cpuid of other cpus
arm: arm64: pmu: Assign platform PMU CPU affinity
arm64: pmu: Detect multiple generic PMUs and append counter
arm64: pmu: Detect and enable multiple PMUs in an ACPI system
arm: pmu: Add PMU definitions for cores not initially online
Mark Salter (1):
arm64: pmu: Cache PMU interrupt numbers from MADT parse
arch/arm/include/asm/cputype.h | 4 +
arch/arm/kernel/setup.c | 2 +-
arch/arm64/include/asm/cputype.h | 3 +
arch/arm64/kernel/perf_event.c | 2 +-
arch/arm64/kernel/smp.c | 18 ++-
drivers/perf/Kconfig | 4 +
drivers/perf/Makefile | 1 +
drivers/perf/arm_pmu.c | 103 +++++++++++++--
drivers/perf/arm_pmu_acpi.c | 271 +++++++++++++++++++++++++++++++++++++++
include/linux/perf/arm_pmu.h | 12 ++
10 files changed, 400 insertions(+), 20 deletions(-)
create mode 100644 drivers/perf/arm_pmu_acpi.c
--
2.5.5
^ permalink raw reply
* [PATCH v12 1/7] arm64: Rename the common MADT parse routine
From: Jeremy Linton @ 2017-01-10 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484068672-15852-1-git-send-email-jeremy.linton@arm.com>
The MADT parser in smp.c is now being used to parse
out NUMA, PMU and ACPI parking protocol information as
well as the GIC information for which it was originally
created. Rename it to avoid a misleading name.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
arch/arm64/kernel/smp.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index cb87234..8ea244c 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -517,13 +517,14 @@ static unsigned int cpu_count = 1;
#ifdef CONFIG_ACPI
/*
- * acpi_map_gic_cpu_interface - parse processor MADT entry
+ * acpi_verify_and_map_madt - parse processor MADT entry
*
* Carry out sanity checks on MADT processor entry and initialize
- * cpu_logical_map on success
+ * cpu_logical_map, the ACPI parking protocol, NUMA mapping
+ * and the PMU interrupts on success
*/
static void __init
-acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
+acpi_verify_and_map_madt(struct acpi_madt_generic_interrupt *processor)
{
u64 hwid = processor->arm_mpidr;
@@ -577,7 +578,7 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
}
static int __init
-acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header,
+acpi_parse_madt_common(struct acpi_subtable_header *header,
const unsigned long end)
{
struct acpi_madt_generic_interrupt *processor;
@@ -588,7 +589,7 @@ acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header,
acpi_table_print_madt_entry(header);
- acpi_map_gic_cpu_interface(processor);
+ acpi_verify_and_map_madt(processor);
return 0;
}
@@ -672,7 +673,7 @@ void __init smp_init_cpus(void)
* we need for SMP init
*/
acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
- acpi_parse_gic_cpu_interface, 0);
+ acpi_parse_madt_common, 0);
if (cpu_count > nr_cpu_ids)
pr_warn("Number of cores (%d) exceeds configured maximum of %d - clipping\n",
--
2.5.5
^ permalink raw reply related
* [PATCH v12 2/7] arm: arm64: Add routine to determine cpuid of other cpus
From: Jeremy Linton @ 2017-01-10 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484068672-15852-1-git-send-email-jeremy.linton@arm.com>
It is helpful if we can read the cpuid/midr of other CPUs
in the system independent of arm/arm64.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
arch/arm/include/asm/cputype.h | 4 ++++
arch/arm/kernel/setup.c | 2 +-
arch/arm64/include/asm/cputype.h | 3 +++
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
index 522b5fe..1e87d06 100644
--- a/arch/arm/include/asm/cputype.h
+++ b/arch/arm/include/asm/cputype.h
@@ -235,6 +235,10 @@ static inline unsigned int __attribute_const__ read_cpuid_mpidr(void)
#define cpu_is_sa1100() (read_cpuid_part() == ARM_CPU_PART_SA1100)
#define cpu_is_sa1110() (read_cpuid_part() == ARM_CPU_PART_SA1110)
+#define read_specific_cpuid(cpu_num) (is_smp() ? \
+ per_cpu_ptr(&cpu_data, cpu_num)->cpuid \
+ : read_cpuid_id())
+
/*
* Intel's XScale3 core supports some v6 features (supersections, L2)
* but advertises itself as v5 as it does not support the v6 ISA. For
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 34e3f3c..d443c97 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1217,7 +1217,7 @@ static int c_show(struct seq_file *m, void *v)
* "processor". Give glibc what it expects.
*/
seq_printf(m, "processor\t: %d\n", i);
- cpuid = is_smp() ? per_cpu(cpu_data, i).cpuid : read_cpuid_id();
+ cpuid = read_specific_cpuid(i);
seq_printf(m, "model name\t: %s rev %d (%s)\n",
cpu_name, cpuid & 15, elf_platform);
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 26a68dd..a6d26e1 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -124,6 +124,9 @@ static inline u32 __attribute_const__ read_cpuid_cachetype(void)
{
return read_cpuid(CTR_EL0);
}
+
+#define read_specific_cpuid(cpu_num) per_cpu_ptr(&cpu_data, cpu_num)->reg_midr
+
#endif /* __ASSEMBLY__ */
#endif
--
2.5.5
^ permalink raw reply related
* [PATCH v12 3/7] arm64: pmu: Cache PMU interrupt numbers from MADT parse
From: Jeremy Linton @ 2017-01-10 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484068672-15852-1-git-send-email-jeremy.linton@arm.com>
From: Mark Salter <msalter@redhat.com>
In the case of ACPI, the PMU IRQ information is contained in the
MADT table. Also, since the PMU does not exist as a device in the
ACPI DSDT table, it is necessary to create a platform device so
that the appropriate driver probing is triggered. Since the platform
device creation needs to happen after the CPU's have been started, and
the MADT parsing needs to happen before, we save off the interrupt
numbers discovered during the parsing.
Signed-off-by: Mark Salter <msalter@redhat.com>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
arch/arm64/kernel/smp.c | 5 +++++
drivers/perf/Kconfig | 4 ++++
drivers/perf/Makefile | 1 +
drivers/perf/arm_pmu_acpi.c | 40 ++++++++++++++++++++++++++++++++++++++++
include/linux/perf/arm_pmu.h | 7 +++++++
5 files changed, 57 insertions(+)
create mode 100644 drivers/perf/arm_pmu_acpi.c
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 8ea244c..cbaab44 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -37,6 +37,7 @@
#include <linux/completion.h>
#include <linux/of.h>
#include <linux/irq_work.h>
+#include <linux/perf/arm_pmu.h>
#include <asm/alternative.h>
#include <asm/atomic.h>
@@ -552,6 +553,7 @@ acpi_verify_and_map_madt(struct acpi_madt_generic_interrupt *processor)
}
bootcpu_valid = true;
early_map_cpu_to_node(0, acpi_numa_get_nid(0, hwid));
+ arm_pmu_parse_acpi(0, processor);
return;
}
@@ -572,6 +574,9 @@ acpi_verify_and_map_madt(struct acpi_madt_generic_interrupt *processor)
*/
acpi_set_mailbox_entry(cpu_count, processor);
+ /* get PMU irq info */
+ arm_pmu_parse_acpi(cpu_count, processor);
+
early_map_cpu_to_node(cpu_count, acpi_numa_get_nid(cpu_count, hwid));
cpu_count++;
diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index 4d5c5f9..697df05 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -19,4 +19,8 @@ config XGENE_PMU
help
Say y if you want to use APM X-Gene SoC performance monitors.
+config ARM_PMU_ACPI
+ def_bool y
+ depends on ARM_PMU && ACPI
+
endmenu
diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index b116e98..d1d7762 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_ARM_PMU) += arm_pmu.o
obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
+obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
new file mode 100644
index 0000000..2008001
--- /dev/null
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -0,0 +1,40 @@
+/*
+ * ARM ACPI PMU support
+ *
+ * Copyright (C) 2015 Red Hat Inc.
+ * Author: Mark Salter <msalter@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include <asm/cpu.h>
+#include <linux/acpi.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
+#include <linux/list.h>
+#include <linux/perf/arm_pmu.h>
+#include <linux/platform_device.h>
+
+struct pmu_irq {
+ int gsi;
+ int trigger;
+ bool used;
+};
+
+static struct pmu_irq pmu_irqs[NR_CPUS];
+
+/*
+ * Called from acpi_verify_and_map_madt()'s MADT parsing during boot.
+ * This routine saves off the GSI's and their trigger state for use when we are
+ * ready to build the PMU platform device.
+ */
+void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)
+{
+ pmu_irqs[cpu].gsi = gic->performance_interrupt;
+ if (gic->flags & ACPI_MADT_PERFORMANCE_IRQ_MODE)
+ pmu_irqs[cpu].trigger = ACPI_EDGE_SENSITIVE;
+ else
+ pmu_irqs[cpu].trigger = ACPI_LEVEL_SENSITIVE;
+}
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 8462da2..df1ba55 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -164,4 +164,11 @@ int arm_pmu_device_probe(struct platform_device *pdev,
#endif /* CONFIG_ARM_PMU */
+#ifdef CONFIG_ARM_PMU_ACPI
+struct acpi_madt_generic_interrupt;
+void arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic);
+#else
+#define arm_pmu_parse_acpi(a, b) do { } while (0)
+#endif /* CONFIG_ARM_PMU_ACPI */
+
#endif /* __ARM_PMU_H__ */
--
2.5.5
^ permalink raw reply related
* [PATCH v12 4/7] arm: arm64: pmu: Assign platform PMU CPU affinity
From: Jeremy Linton @ 2017-01-10 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484068672-15852-1-git-send-email-jeremy.linton@arm.com>
On systems with multiple PMU types the PMU to CPU affinity
needs to be detected and set. The CPU to interrupt affinity
should also be set.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
drivers/perf/arm_pmu.c | 43 +++++++++++++++++++++++++++++++++----------
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 6d93358..37e241f 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -11,6 +11,7 @@
*/
#define pr_fmt(fmt) "hw perfevents: " fmt
+#include <linux/acpi.h>
#include <linux/bitmap.h>
#include <linux/cpumask.h>
#include <linux/cpu_pm.h>
@@ -24,6 +25,7 @@
#include <linux/irq.h>
#include <linux/irqdesc.h>
+#include <asm/cpu.h>
#include <asm/cputype.h>
#include <asm/irq_regs.h>
@@ -889,25 +891,47 @@ static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
}
/*
- * CPU PMU identification and probing.
+ * CPU PMU identification and probing. Its possible to have
+ * multiple CPU types in an ARM machine. Assure that we are
+ * picking the right PMU types based on the CPU in question
*/
-static int probe_current_pmu(struct arm_pmu *pmu,
- const struct pmu_probe_info *info)
+static int probe_plat_pmu(struct arm_pmu *pmu,
+ const struct pmu_probe_info *info,
+ unsigned int pmuid)
{
- int cpu = get_cpu();
- unsigned int cpuid = read_cpuid_id();
int ret = -ENODEV;
+ int cpu;
+ int aff_ctr = 0;
+ struct platform_device *pdev = pmu->plat_device;
+ int irq = platform_get_irq(pdev, 0);
- pr_info("probing PMU on CPU %d\n", cpu);
+ if (irq >= 0 && !irq_is_percpu(irq)) {
+ pmu->irq_affinity = kcalloc(pdev->num_resources, sizeof(int),
+ GFP_KERNEL);
+ if (!pmu->irq_affinity)
+ return -ENOMEM;
+ }
+
+ for_each_possible_cpu(cpu) {
+ unsigned int cpuid = read_specific_cpuid(cpu);
+
+ if (cpuid == pmuid) {
+ cpumask_set_cpu(cpu, &pmu->supported_cpus);
+ if (pmu->irq_affinity) {
+ pmu->irq_affinity[aff_ctr] = cpu;
+ aff_ctr++;
+ }
+ }
+ }
+ /* find the type of PMU given the CPU */
for (; info->init != NULL; info++) {
- if ((cpuid & info->mask) != info->cpuid)
+ if ((pmuid & info->mask) != info->cpuid)
continue;
ret = info->init(pmu);
break;
}
- put_cpu();
return ret;
}
@@ -1043,8 +1067,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,
if (!ret)
ret = init_fn(pmu);
} else if (probe_table) {
- cpumask_setall(&pmu->supported_cpus);
- ret = probe_current_pmu(pmu, probe_table);
+ ret = probe_plat_pmu(pmu, probe_table, read_cpuid_id());
}
if (ret) {
--
2.5.5
^ permalink raw reply related
* [PATCH v12 5/7] arm64: pmu: Detect multiple generic PMUs and append counter
From: Jeremy Linton @ 2017-01-10 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484068672-15852-1-git-send-email-jeremy.linton@arm.com>
In heterogeneous CPU systems its likely that there are multiple
PMU types. If a system is using the generic armv8_pmuv3 rather
than a PMU with a hard-coded set of events then we want to uniquely
identify each PMU in /sys. We do this by appending an "_x" to the
pmu name. This then creates PMUs like, "armv8_pmuv3" and
"armv8_pmuv3_1", "armv8_pmuv3_2" for a system with 3 PMU types.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
arch/arm64/kernel/perf_event.c | 2 +-
drivers/perf/arm_pmu.c | 20 ++++++++++++++++++++
include/linux/perf/arm_pmu.h | 1 +
3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 57ae9d9..0fbd7ef 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -1002,7 +1002,7 @@ static void armv8_pmu_init(struct arm_pmu *cpu_pmu)
static int armv8_pmuv3_init(struct arm_pmu *cpu_pmu)
{
armv8_pmu_init(cpu_pmu);
- cpu_pmu->name = "armv8_pmuv3";
+ cpu_pmu->name = ARMV8_PMUV3_DESCRIPTION;
cpu_pmu->map_event = armv8_pmuv3_map_event;
cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_EVENTS] =
&armv8_pmuv3_events_attr_group;
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 37e241f..85566f6 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -1035,6 +1035,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,
const struct of_device_id *of_table,
const struct pmu_probe_info *probe_table)
{
+ static int duplicate_pmus;
const struct of_device_id *of_id;
const int (*init_fn)(struct arm_pmu *);
struct device_node *node = pdev->dev.of_node;
@@ -1075,6 +1076,25 @@ int arm_pmu_device_probe(struct platform_device *pdev,
goto out_free;
}
+ /*
+ * if this pmu declaration is a generic pmu and we have
+ * previously found a generic pmu on this platform
+ * then append a PMU number to the pmu name. This avoids
+ * changing the names of PMUs that are specific to a class
+ * of CPUs. The assumption is that if we match a specific PMU
+ * then it's unique, and another PMU in the system will match
+ * a different entry rather than needing the _number to
+ * assure its unique.
+ */
+ if (!strcmp(pmu->name, ARMV8_PMUV3_DESCRIPTION)) {
+ if (duplicate_pmus) {
+ pmu->name = kasprintf(GFP_KERNEL, "%s_%d",
+ pmu->name, duplicate_pmus);
+ if (!pmu->name)
+ goto out_free;
+ }
+ duplicate_pmus++;
+ }
ret = cpu_pmu_init(pmu);
if (ret)
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index df1ba55..42b5edb 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -161,6 +161,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,
const struct pmu_probe_info *probe_table);
#define ARMV8_PMU_PDEV_NAME "armv8-pmu"
+#define ARMV8_PMUV3_DESCRIPTION "armv8_pmuv3"
#endif /* CONFIG_ARM_PMU */
--
2.5.5
^ permalink raw reply related
* [PATCH v12 6/7] arm64: pmu: Detect and enable multiple PMUs in an ACPI system
From: Jeremy Linton @ 2017-01-10 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484068672-15852-1-git-send-email-jeremy.linton@arm.com>
Its possible that an ACPI system has multiple CPU types in it
with differing PMU counters. Iterate the CPU's and make a determination
about how many of each type exist in the system. Then take and create
a PMU platform device for each type, and assign it the interrupts parsed
from the MADT. Creating a platform device is necessary because the PMUs
are not described as devices in the DSDT table.
This code is loosely based on earlier work by Mark Salter.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
drivers/perf/arm_pmu.c | 8 +-
drivers/perf/arm_pmu_acpi.c | 231 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 238 insertions(+), 1 deletion(-)
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 85566f6..77ec1ae 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -1068,7 +1068,13 @@ int arm_pmu_device_probe(struct platform_device *pdev,
if (!ret)
ret = init_fn(pmu);
} else if (probe_table) {
- ret = probe_plat_pmu(pmu, probe_table, read_cpuid_id());
+ if (acpi_disabled) {
+ /* use the current cpu. */
+ ret = probe_plat_pmu(pmu, probe_table,
+ read_cpuid_id());
+ } else {
+ ret = probe_plat_pmu(pmu, probe_table, pdev->id);
+ }
}
if (ret) {
diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
index 2008001..e4e107c 100644
--- a/drivers/perf/arm_pmu_acpi.c
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -2,13 +2,17 @@
* ARM ACPI PMU support
*
* Copyright (C) 2015 Red Hat Inc.
+ * Copyright (C) 2016 ARM Ltd.
* Author: Mark Salter <msalter@redhat.com>
+ * Jeremy Linton <jeremy.linton@arm.com>
*
* This work is licensed under the terms of the GNU GPL, version 2. See
* the COPYING file in the top-level directory.
*
*/
+#define pr_fmt(fmt) "ACPI-PMU: " fmt
+
#include <asm/cpu.h>
#include <linux/acpi.h>
#include <linux/irq.h>
@@ -20,9 +24,16 @@
struct pmu_irq {
int gsi;
int trigger;
+ int irq;
bool used;
};
+struct pmu_types {
+ struct list_head list;
+ int cpu_type;
+ int cpu_count;
+};
+
static struct pmu_irq pmu_irqs[NR_CPUS];
/*
@@ -38,3 +49,223 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)
else
pmu_irqs[cpu].trigger = ACPI_LEVEL_SENSITIVE;
}
+
+static void __init arm_pmu_acpi_handle_alloc_failure(struct list_head *pmus)
+{
+ int i;
+ struct pmu_types *pmu, *safe_temp;
+
+ list_for_each_entry_safe(pmu, safe_temp, pmus, list) {
+ list_del(&pmu->list);
+ kfree(pmu);
+ }
+
+ for_each_possible_cpu(i)
+ if (pmu_irqs[i].irq > 0)
+ acpi_unregister_gsi(pmu_irqs[i].gsi);
+}
+
+/*
+ * Count number and type of CPU cores in the system. Returns the number
+ * of "unused" MADT entries we could not associate with a PMU. This can
+ * be the result of CPU's not being online, or errors in the MADT.
+ * Under normal circumstances this will be 0.
+ */
+static int __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
+{
+ int i;
+ int unused_madt_entries = 0;
+
+ for_each_possible_cpu(i) {
+ u32 reg_midr = read_specific_cpuid(i);
+ struct pmu_types *pmu;
+
+ /*
+ * Ignore GSI registration failure for now, as
+ * some of the MADT entries may not be used.
+ */
+ pmu_irqs[i].irq = acpi_register_gsi(NULL, pmu_irqs[i].gsi,
+ pmu_irqs[i].trigger,
+ ACPI_ACTIVE_HIGH);
+ /* likely not online */
+ if (reg_midr == 0) {
+ unused_madt_entries++;
+ continue;
+ }
+
+ list_for_each_entry(pmu, pmus, list) {
+ if (pmu->cpu_type == reg_midr) {
+ pmu->cpu_count++;
+ break;
+ }
+ }
+
+ /* we didn't find the CPU type, add an entry to identify it */
+ if (&pmu->list == pmus) {
+ pmu = kzalloc(sizeof(struct pmu_types), GFP_KERNEL);
+ if (!pmu) {
+ pr_err("Unable to allocate pmu_types\n");
+ arm_pmu_acpi_handle_alloc_failure(pmus);
+ break;
+ }
+ pmu->cpu_type = reg_midr;
+ pmu->cpu_count++;
+ list_add_tail(&pmu->list, pmus);
+ }
+ }
+
+ return unused_madt_entries;
+}
+
+static int __init arm_pmu_acpi_register_device(int count, struct resource *res,
+ int cpu_id)
+{
+ struct platform_device *pdev;
+ int err = -ENOMEM;
+
+ pdev = platform_device_alloc(ARMV8_PMU_PDEV_NAME, cpu_id);
+ if (pdev) {
+ err = platform_device_add_resources(pdev, res, count);
+ if (!err)
+ err = platform_device_add(pdev);
+ if (err) {
+ pr_warn("Unable to register PMU device\n");
+ platform_device_put(pdev);
+ }
+ } else {
+ pr_warn("Unable to allocate platform device\n");
+ }
+
+ return err;
+}
+
+static void __init arm_pmu_acpi_unregister_pmu_gsi(int cpu_id)
+{
+ int i;
+
+ for_each_possible_cpu(i) {
+
+ if (read_specific_cpuid(i) == cpu_id) {
+ pmu_irqs[i].used = false;
+ if (pmu_irqs[i].irq > 0)
+ acpi_unregister_gsi(pmu_irqs[i].gsi);
+ pmu_irqs[i].gsi = -ENODEV;
+ }
+ }
+}
+
+/*
+ * Registers the group of PMU interfaces which correspond to the 'cpu_id'.
+ * This group utilizes 'count' resources in the 'res'.
+ */
+static int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
+ int cpu_id)
+{
+ int err;
+
+ err = arm_pmu_acpi_register_device(count, res, cpu_id);
+
+ /* unmark and unregister GSIs for this PMU */
+ if (err)
+ arm_pmu_acpi_unregister_pmu_gsi(cpu_id);
+
+ return err;
+}
+
+int arm_pmu_acpi_retrieve_irq(struct resource *res, int cpu)
+{
+ int irq = -ENODEV;
+
+ if (pmu_irqs[cpu].used) {
+ pr_info("CPU %d's interrupt is already used\n", cpu);
+ } else {
+ pmu_irqs[cpu].used = true;
+ res->start = pmu_irqs[cpu].irq;
+ res->end = pmu_irqs[cpu].irq;
+ res->flags = IORESOURCE_IRQ;
+ if (pmu_irqs[cpu].trigger == ACPI_EDGE_SENSITIVE)
+ res->flags |= IORESOURCE_IRQ_HIGHEDGE;
+ else
+ res->flags |= IORESOURCE_IRQ_HIGHLEVEL;
+ }
+ return irq;
+}
+
+/*
+ * For the given cpu/pmu type, walk all known GSIs, register them, and add
+ * them to the resource structure. Return the number of GSI's contained
+ * in the res structure, and the id of the last CPU/PMU we added.
+ */
+static int __init arm_pmu_acpi_gsi_res(struct pmu_types *pmus,
+ struct resource *res)
+{
+ int i, count;
+
+ /* lets group all the PMU's from similar CPU's together */
+ count = 0;
+ for_each_possible_cpu(i) {
+ u32 reg_midr = read_specific_cpuid(i);
+
+ if (pmus->cpu_type == reg_midr) {
+ if ((pmu_irqs[i].gsi == 0) && (reg_midr != 0))
+ continue;
+
+ /* likely not online */
+ if (!reg_midr)
+ continue;
+
+ arm_pmu_acpi_retrieve_irq(&res[count], i);
+ count++;
+ }
+ }
+ return count;
+}
+
+static int __init pmu_acpi_register(struct pmu_types *pmu)
+{
+ int count;
+ int err = -ENOMEM;
+ struct resource *res;
+
+ res = kcalloc(pmu->cpu_count, sizeof(struct resource), GFP_KERNEL);
+
+ /* for a given PMU type, collect all the GSIs. */
+ if (res) {
+ count = arm_pmu_acpi_gsi_res(pmu, res);
+ /* register this set of interrupts with a new PMU device */
+ err = arm_pmu_acpi_register_pmu(pmu->cpu_count, res,
+ pmu->cpu_type);
+ if (!err)
+ pr_info("Register %d devices for %X\n", count,
+ pmu->cpu_type);
+ kfree(res);
+ } else {
+ pr_warn("PMU unable to allocate interrupt resource\n");
+ arm_pmu_acpi_unregister_pmu_gsi(pmu->cpu_type);
+ }
+ return err;
+}
+
+static int __init pmu_acpi_init(void)
+{
+ struct pmu_types *pmu, *safe_temp;
+ bool unused_madt_entries;
+ LIST_HEAD(pmus);
+
+ if (acpi_disabled)
+ return 0;
+
+ unused_madt_entries = arm_pmu_acpi_determine_cpu_types(&pmus);
+
+ list_for_each_entry_safe(pmu, safe_temp, &pmus, list) {
+ pmu->cpu_count += unused_madt_entries;
+ pmu_acpi_register(pmu);
+
+ list_del(&pmu->list);
+ kfree(pmu);
+ }
+
+ return 0;
+}
+
+arch_initcall(pmu_acpi_init);
--
2.5.5
^ permalink raw reply related
* [PATCH v12 7/7] arm: pmu: Add PMU definitions for cores not initially online
From: Jeremy Linton @ 2017-01-10 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484068672-15852-1-git-send-email-jeremy.linton@arm.com>
ACPI CPUs aren't associated with a PMU until they have been put
online. This means that we potentially have to update a PMU
definition the first time a CPU is hot added to the machine.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
drivers/perf/arm_pmu.c | 34 ++++++++++++++++++++++++++++++++--
include/linux/perf/arm_pmu.h | 4 ++++
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 77ec1ae..cf5122f 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -711,6 +711,26 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
return 0;
}
+static void arm_perf_associate_new_cpu(struct arm_pmu *lpmu, unsigned int cpu)
+{
+ struct platform_device *pdev = lpmu->plat_device;
+ struct resource *res;
+ struct pmu_hw_events *events;
+ int num_res;
+
+ for (num_res = 0; num_res < pdev->num_resources; num_res++) {
+ if (!pdev->resource[num_res].flags)
+ break;
+ }
+ res = &pdev->resource[num_res];
+ arm_pmu_acpi_retrieve_irq(res, cpu);
+ events = per_cpu_ptr(lpmu->hw_events, cpu);
+ cpumask_set_cpu(cpu, &lpmu->supported_cpus);
+ if (lpmu->irq_affinity)
+ lpmu->irq_affinity[num_res] = cpu;
+ events->percpu_pmu = lpmu;
+}
+
/*
* PMU hardware loses all context when a CPU goes offline.
* When a CPU is hotplugged back in, since some hardware registers are
@@ -721,10 +741,18 @@ static int arm_perf_starting_cpu(unsigned int cpu, struct hlist_node *node)
{
struct arm_pmu *pmu = hlist_entry_safe(node, struct arm_pmu, node);
- if (!cpumask_test_cpu(cpu, &pmu->supported_cpus))
- return 0;
+ if (!cpumask_test_cpu(cpu, &pmu->supported_cpus)) {
+ unsigned int cpuid = read_specific_cpuid(cpu);
+
+ if (acpi_disabled)
+ return 0;
+ if (cpuid != pmu->id)
+ return 0;
+ arm_perf_associate_new_cpu(pmu, cpu);
+ }
if (pmu->reset)
pmu->reset(pmu);
+
return 0;
}
@@ -905,6 +933,8 @@ static int probe_plat_pmu(struct arm_pmu *pmu,
struct platform_device *pdev = pmu->plat_device;
int irq = platform_get_irq(pdev, 0);
+ pmu->id = pmuid;
+
if (irq >= 0 && !irq_is_percpu(irq)) {
pmu->irq_affinity = kcalloc(pdev->num_resources, sizeof(int),
GFP_KERNEL);
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 42b5edb..f652cd1 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -112,6 +112,7 @@ struct arm_pmu {
struct mutex reserve_mutex;
u64 max_period;
bool secure_access; /* 32-bit ARM only */
+ unsigned int id;
#define ARMV8_PMUV3_MAX_COMMON_EVENTS 0x40
DECLARE_BITMAP(pmceid_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS);
struct platform_device *plat_device;
@@ -168,8 +169,11 @@ int arm_pmu_device_probe(struct platform_device *pdev,
#ifdef CONFIG_ARM_PMU_ACPI
struct acpi_madt_generic_interrupt;
void arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic);
+int arm_pmu_acpi_retrieve_irq(struct resource *pdev, int cpu);
#else
#define arm_pmu_parse_acpi(a, b) do { } while (0)
+#define arm_pmu_acpi_retrieve_irq(pdev, cpu) \
+ do { } while (0)
#endif /* CONFIG_ARM_PMU_ACPI */
#endif /* __ARM_PMU_H__ */
--
2.5.5
^ permalink raw reply related
* [PATCH 1/3] ARM: at91: flush the L2 cache before entering cpu idle
From: Alexandre Belloni @ 2017-01-10 17:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACh+v5NX9Da__zXtnVGgbO9wYAzEJy-8rjVPHdQx_F6QDk+xZg@mail.gmail.com>
On 10/01/2017 at 17:50:58 +0100, Jean-Jacques Hiblot wrote :
> 2017-01-10 17:18 GMT+01:00 Alexandre Belloni
> <alexandre.belloni@free-electrons.com>:
> > I though a bit more about it, and I don't really like the new compatible
> > string. I don't feel this should be necessary.
> >
> > What about the following:
> >
> > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> > index b4332b727e9c..0333aca63e44 100644
> > --- a/arch/arm/mach-at91/pm.c
> > +++ b/arch/arm/mach-at91/pm.c
> > @@ -53,6 +53,7 @@ extern void at91_pinctrl_gpio_resume(void);
> > static struct {
> > unsigned long uhp_udp_mask;
> > int memctrl;
> > + bool has_l2_cache;
> > } at91_pm_data;
> >
> > void __iomem *at91_ramc_base[2];
> > @@ -267,6 +268,11 @@ static void at91_ddr_standby(void)
> > u32 lpr0, lpr1 = 0;
> > u32 saved_lpr0, saved_lpr1 = 0;
> >
>
> > + if (at91_pm_data.has_l2_cache) {
> > + flush_cache_all();
> what is the point of calling flush_cache_all() here ? Do we really
> care that dirty data in L1 is written to DDR ? I may be missing
> something but to me it's just extra latency.
I agree that this one is the main problem.
> > + outer_disable();
> It seems to me that if there's no L2 cache, then outer_disable() is a
> no-op. It could be called unconditionally.
It is not on sama5, it will jump to outer_disable which will at least
save the context and restore it
> > + }
> > +
> > if (at91_ramc_base[1]) {
> > saved_lpr1 = at91_ramc_read(1, AT91_DDRSDRC_LPR);
> > lpr1 = saved_lpr1 & ~AT91_DDRSDRC_LPCB;
> > @@ -287,6 +293,9 @@ static void at91_ddr_standby(void)
> > at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr0);
> > if (at91_ramc_base[1])
> > at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
> > +
> > + if (at91_pm_data.has_l2_cache)
> > + outer_resume();
>
> same remark as for outer_disable()
It is not either but this is a macro and I admit testing has_l2_cache is
superfluous.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v2 2/2] media: rc: add driver for IR remote receiver on MT7623 SoC
From: Sean Young @ 2017-01-10 17:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484056789.4057.17.camel@mtkswgap22>
Hi Sean,
The driver is looking very good, we are looking at minor details now.
On Tue, Jan 10, 2017 at 09:59:49PM +0800, Sean Wang wrote:
> On Tue, 2017-01-10 at 11:09 +0000, Sean Young wrote:
>
> > > +#include <linux/clk.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_platform.h>
> > > +#include <linux/reset.h>
> > > +#include <media/rc-core.h>
> > > +
> > > +#define MTK_IR_DEV KBUILD_MODNAME
> >
> > You could remove this #define and just use KBUILD_MODNAME
>
> I preferred to use MTK_IR_DEV internally that helps
> renaming in the future if necessary.
ok.
> >
> > > +
> > > +/* Register to enable PWM and IR */
> > > +#define MTK_CONFIG_HIGH_REG 0x0c
> > > +/* Enable IR pulse width detection */
> > > +#define MTK_PWM_EN BIT(13)
> > > +/* Enable IR hardware function */
> > > +#define MTK_IR_EN BIT(0)
> > > +
> > > +/* Register to setting sample period */
> > > +#define MTK_CONFIG_LOW_REG 0x10
> > > +/* Field to set sample period */
> > > +#define CHK_PERIOD DIV_ROUND_CLOSEST(MTK_IR_SAMPLE, \
> > > + MTK_IR_CLK_PERIOD)
> > > +#define MTK_CHK_PERIOD (((CHK_PERIOD) << 8) & (GENMASK(20, 8)))
> > > +#define MTK_CHK_PERIOD_MASK (GENMASK(20, 8))
> > > +
> > > +/* Register to clear state of state machine */
> > > +#define MTK_IRCLR_REG 0x20
> > > +/* Bit to restart IR receiving */
> > > +#define MTK_IRCLR BIT(0)
> > > +
> > > +/* Register containing pulse width data */
> > > +#define MTK_CHKDATA_REG(i) (0x88 + 4 * (i))
> > > +#define MTK_WIDTH_MASK (GENMASK(7, 0))
> > > +
> > > +/* Register to enable IR interrupt */
> > > +#define MTK_IRINT_EN_REG 0xcc
> > > +/* Bit to enable interrupt */
> > > +#define MTK_IRINT_EN BIT(0)
> > > +
> > > +/* Register to ack IR interrupt */
> > > +#define MTK_IRINT_CLR_REG 0xd0
> > > +/* Bit to clear interrupt status */
> > > +#define MTK_IRINT_CLR BIT(0)
> > > +
> > > +/* Maximum count of samples */
> > > +#define MTK_MAX_SAMPLES 0xff
> > > +/* Indicate the end of IR message */
> > > +#define MTK_IR_END(v, p) ((v) == MTK_MAX_SAMPLES && (p) == 0)
> > > +/* Number of registers to record the pulse width */
> > > +#define MTK_CHKDATA_SZ 17
> > > +/* Source clock frequency */
> > > +#define MTK_IR_BASE_CLK 273000000
> > > +/* Frequency after IR internal divider */
> > > +#define MTK_IR_CLK_FREQ (MTK_IR_BASE_CLK / 4)
>
> > > +static irqreturn_t mtk_ir_irq(int irqno, void *dev_id)
> > > +{
> > > + struct mtk_ir *ir = dev_id;
> > > + u8 wid = 0;
> > > + u32 i, j, val;
> > > + DEFINE_IR_RAW_EVENT(rawir);
> > > +
> > > + mtk_irq_disable(ir, MTK_IRINT_EN);
> >
> > The kernel guarantees that calls to the interrupt handler are serialised,
> > no need to disable the interrupt in the handler.
>
> agreed. I will save the mtk irq disable/enable and retest again.
>
>
> > > +
> > > + /* Reset decoder state machine */
> > > + ir_raw_event_reset(ir->rc);
> >
> > Not needed.
>
>
> two reasons I added the line here
>
> 1)
> I thought it is possible the decoder goes to the
> middle state when getting the data not belonged
> to the protocol. If so, that would cause the decoding
> fails in the next time receiving the valid protocol data.
The last IR event submitted will always be a long space, that's enough
to reset the decoders. Adding a ir_raw_event_reset() will do this
more explicitly, rather than their state machines resetting themselves
through the trailing space.
> 2)
> the mtk hardware register always contains the start of
> IR message. So force to sync the state between
> HW and ir-core.
>
>
>
> > > +
> > > + /* First message must be pulse */
> > > + rawir.pulse = false;
> >
> > pulse = true?
>
> becasue of rawir.pulse = !rawir.pulse does as below
> so the initial value is set as false.
Ah, sorry, of course. :)
> > > +
> > > + /* Handle all pulse and space IR controller captures */
> > > + for (i = 0 ; i < MTK_CHKDATA_SZ ; i++) {
> > > + val = mtk_r32(ir, MTK_CHKDATA_REG(i));
> > > + dev_dbg(ir->dev, "@reg%d=0x%08x\n", i, val);
> > > +
> > > + for (j = 0 ; j < 4 ; j++) {
> > > + wid = (val & (MTK_WIDTH_MASK << j * 8)) >> j * 8;
> > > + rawir.pulse = !rawir.pulse;
> > > + rawir.duration = wid * (MTK_IR_SAMPLE + 1);
> > > + ir_raw_event_store_with_filter(ir->rc, &rawir);
> > > + }
> >
> > In v1 you would break out of the loop if the ir message was shorter, but
> > now you are always passing on 68 pulses and spaces. Is that right?
>
> as I asked in the previous mail list as below i copied from it, so i
> made some changes ...
>
> """""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
> > I had another question. I found multiple and same IR messages being
> > received when using SONY remote controller. Should driver needs to
> > report each message or only one of these to the upper layer ?
>
> In general the driver shouldn't try to change any IR message, this
> should be done in rc-core if necessary.
>
> rc-core should handle this correctly. If the same key is received twice
> within IR_KEYPRESS_TIMEOUT (250ms) then it not reported to the input
> layer.
> """"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
>
> for example:
> the 68 pulse/spaces might contains 2.x IR messages when I
> pressed one key on SONY remote control.
>
> the v1 proposed is passing only one IR message into ir-core ;
> the v2 done is passing all IR messages even including the last
> incomplete message into ir-core.
Yes, agreed. Sorry if I wasn't clear. I just wanted to make sure you've
thought about what happens when the IR message is short (e.g. rc-5 with
23 pulse-spaces). Are the remaining registers 0 or do we get stale data
from a previous transmit?
> But I was still afraid the state machine can't go back to initial state
> after receiving these incomplete data.
>
> So the ir_raw_event_reset() call in the beginning of ISR seems becoming
> more important.
>
> > > + }
> > > +
> > > + /* The maximum number of edges the IR controller can
> > > + * hold is MTK_CHKDATA_SZ * 4. So if received IR messages
> > > + * is over the limit, the last incomplete IR message would
> > > + * be appended trailing space and still would be sent into
> > > + * ir-rc-raw to decode. That helps it is possible that it
> > > + * has enough information to decode a scancode even if the
> > > + * trailing end of the message is missing.
> > > + */
> > > + if (!MTK_IR_END(wid, rawir.pulse)) {
> > > + rawir.pulse = false;
> > > + rawir.duration = MTK_MAX_SAMPLES * (MTK_IR_SAMPLE + 1);
> > > + ir_raw_event_store_with_filter(ir->rc, &rawir);
> > > + }
See here you add a long space if one was not added already.
> > > +
> > > + ir_raw_event_handle(ir->rc);
> > > +
> > > + /* Restart controller for the next receive */
> > > + mtk_w32_mask(ir, 0x1, MTK_IRCLR, MTK_IRCLR_REG);
> > > +
> > > + /* Clear interrupt status */
> > > + mtk_w32_mask(ir, 0x1, MTK_IRINT_CLR, MTK_IRINT_CLR_REG);
> > > +
> > > + /* Enable interrupt */
> > > + mtk_irq_enable(ir, MTK_IRINT_EN);
> > > +
> > > + return IRQ_HANDLED;
> > > +}
> > > +
> > > +static int mtk_ir_probe(struct platform_device *pdev)
> > > +{
> > > + struct device *dev = &pdev->dev;
> > > + struct device_node *dn = dev->of_node;
> > > + struct resource *res;
> > > + struct mtk_ir *ir;
> > > + u32 val;
> > > + int ret = 0;
> > > + const char *map_name;
> > > +
> > > + ir = devm_kzalloc(dev, sizeof(struct mtk_ir), GFP_KERNEL);
> > > + if (!ir)
> > > + return -ENOMEM;
> > > +
> > > + ir->dev = dev;
> > > +
> > > + if (!of_device_is_compatible(dn, "mediatek,mt7623-cir"))
> > > + return -ENODEV;
> > > +
> > > + ir->clk = devm_clk_get(dev, "clk");
> > > + if (IS_ERR(ir->clk)) {
> > > + dev_err(dev, "failed to get a ir clock.\n");
> > > + return PTR_ERR(ir->clk);
> > > + }
> > > +
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + ir->base = devm_ioremap_resource(dev, res);
> > > + if (IS_ERR(ir->base)) {
> > > + dev_err(dev, "failed to map registers\n");
> > > + return PTR_ERR(ir->base);
> > > + }
> > > +
> > > + ir->rc = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
> > > + if (!ir->rc) {
> > > + dev_err(dev, "failed to allocate device\n");
> > > + return -ENOMEM;
> > > + }
> > > +
> > > + ir->rc->priv = ir;
> > > + ir->rc->input_name = MTK_IR_DEV;
> > > + ir->rc->input_phys = MTK_IR_DEV "/input0";
> > > + ir->rc->input_id.bustype = BUS_HOST;
> > > + ir->rc->input_id.vendor = 0x0001;
> > > + ir->rc->input_id.product = 0x0001;
> > > + ir->rc->input_id.version = 0x0001;
> > > + map_name = of_get_property(dn, "linux,rc-map-name", NULL);
> > > + ir->rc->map_name = map_name ?: RC_MAP_EMPTY;
> > > + ir->rc->dev.parent = dev;
> > > + ir->rc->driver_name = MTK_IR_DEV;
> > > + ir->rc->allowed_protocols = RC_BIT_ALL;
> > > + ir->rc->rx_resolution = MTK_IR_SAMPLE;
> > > + ir->rc->timeout = MTK_MAX_SAMPLES * (MTK_IR_SAMPLE + 1);
> > > +
> > > + ret = devm_rc_register_device(dev, ir->rc);
> >
> > Here you do devm_rc_register_device()
>
> does it have problem ?
Sorry, no. I just wanted to highlight wrt a comment below.
>
>
> > > + if (ret) {
> > > + dev_err(dev, "failed to register rc device\n");
> > > + return ret;
> > > + }
> > > +
> > > + platform_set_drvdata(pdev, ir);
> > > +
> > > + ir->irq = platform_get_irq(pdev, 0);
> > > + if (ir->irq < 0) {
> > > + dev_err(dev, "no irq resource\n");
> > > + return -ENODEV;
> > > + }
> > > +
> > > + /* Enable interrupt after proper hardware
> > > + * setup and IRQ handler registration
> > > + */
> > > + if (clk_prepare_enable(ir->clk)) {
> > > + dev_err(dev, "try to enable ir_clk failed\n");
> > > + ret = -EINVAL;
> > > + goto exit_clkdisable_clk;
> > > + }
> > > +
> > > + mtk_irq_disable(ir, MTK_IRINT_EN);
> > > +
> > > + ret = devm_request_irq(dev, ir->irq, mtk_ir_irq, 0, MTK_IR_DEV, ir);
> > > + if (ret) {
> > > + dev_err(dev, "failed request irq\n");
> > > + goto exit_clkdisable_clk;
> > > + }
> > > +
> > > + /* Enable IR and PWM */
> > > + val = mtk_r32(ir, MTK_CONFIG_HIGH_REG);
> > > + val |= MTK_PWM_EN | MTK_IR_EN;
> > > + mtk_w32(ir, val, MTK_CONFIG_HIGH_REG);
> > > +
> > > + /* Setting sample period */
> > > + mtk_w32_mask(ir, MTK_CHK_PERIOD, MTK_CHK_PERIOD_MASK,
> > > + MTK_CONFIG_LOW_REG);
> > > +
> > > + mtk_irq_enable(ir, MTK_IRINT_EN);
> > > +
> > > + dev_info(dev, "Initialized MT7623 IR driver, sample period = %luus\n",
> > > + DIV_ROUND_CLOSEST(MTK_IR_SAMPLE, 1000));
> > > +
> > > + return 0;
> > > +
> > > +exit_clkdisable_clk:
> > > + clk_disable_unprepare(ir->clk);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int mtk_ir_remove(struct platform_device *pdev)
> > > +{
> > > + struct mtk_ir *ir = platform_get_drvdata(pdev);
> > > +
> > > + /* Avoid contention between remove handler and
> > > + * IRQ handler so that disabling IR interrupt and
> > > + * waiting for pending IRQ handler to complete
> > > + */
> > > + mtk_irq_disable(ir, MTK_IRINT_EN);
> > > + synchronize_irq(ir->irq);
> > > +
> > > + clk_disable_unprepare(ir->clk);
> > > +
> > > + rc_unregister_device(ir->rc);
> >
> > Yet here you explicitly call rc_unregister_device(). Since it was registered
> > with the devm call, this call is not needed and will lead to double frees etc
>
> bug :( . I will fix it ..
>
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id mtk_ir_match[] = {
> > > + { .compatible = "mediatek,mt7623-cir" },
> > > + {},
> > > +};
> > > +MODULE_DEVICE_TABLE(of, mtk_ir_match);
> > > +
> > > +static struct platform_driver mtk_ir_driver = {
> > > + .probe = mtk_ir_probe,
> > > + .remove = mtk_ir_remove,
> > > + .driver = {
> > > + .name = MTK_IR_DEV,
> > > + .of_match_table = mtk_ir_match,
> > > + },
> > > +};
> > > +
> > > +module_platform_driver(mtk_ir_driver);
> > > +
> > > +MODULE_DESCRIPTION("Mediatek IR Receiver Controller Driver");
> > > +MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
> > > +MODULE_LICENSE("GPL");
> > > --
> > > 2.7.4
> > >
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [RFC 6/8] KVM: arm/arm64: Update the physical timer interrupt level
From: Jintack Lim @ 2017-01-10 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170109121445.GF4348@cbox>
On Mon, Jan 9, 2017 at 7:14 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Mon, Dec 26, 2016 at 12:12:04PM -0500, Jintack Lim wrote:
>> Now that we maintain the EL1 physical timer register states of the VM,
>> update the physical timer interrupt level along with the virtual one.
>>
>> Note that the emulated EL1 physical timer is not mapped to any hardware
>> timer, so we let vgic know that.
>>
>> With this commit, VMs are able to get the physical timer interrupts
>> while they are runnable. But they won't get interrupts once vcpus go to
>> sleep since we don't have code to wake vcpus up on the emulated physical
>> timer expiration yet.
>
> I'm not sure I understand. It looks to me like it's the other way
> around and that this code supports waking up a sleeping VCPU sooner if
> it has a physical timer scheduled, but doesn't yet support causing an
> early exit from running the VPCU base on programing the physical timer?
>
Ah, you are right. This was really two commits: one that checks the
physical timer expiration on entry to the VM AND another one that sets
up the background timer when the VM goes to sleep.
I'll split this into two and put the correct commit messages.
Thanks,
Jintack
>
> Thanks,
> -Christoffer
>
>>
>> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
>> ---
>> arch/arm/kvm/arm.c | 3 +-
>> virt/kvm/arm/arch_timer.c | 76 ++++++++++++++++++++++++++++++++++++++---------
>> 2 files changed, 64 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index 37d1623..d2dfa32 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -295,7 +295,8 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>>
>> int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
>> {
>> - return kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu));
>> + return kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu)) ||
>> + kvm_timer_should_fire(vcpu, vcpu_ptimer(vcpu));
>> }
>>
>> void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
>> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
>> index ed80864..aa7e243 100644
>> --- a/virt/kvm/arm/arch_timer.c
>> +++ b/virt/kvm/arm/arch_timer.c
>> @@ -91,7 +91,8 @@ static void kvm_timer_inject_irq_work(struct work_struct *work)
>> vcpu = container_of(work, struct kvm_vcpu, arch.timer_cpu.expired);
>> vcpu->arch.timer_cpu.armed = false;
>>
>> - WARN_ON(!kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu)));
>> + WARN_ON(!kvm_timer_should_fire(vcpu, vcpu_vtimer(vcpu)) &&
>> + !kvm_timer_should_fire(vcpu, vcpu_ptimer(vcpu)));
>>
>> /*
>> * If the vcpu is blocked we want to wake it up so that it will see
>> @@ -130,6 +131,33 @@ static u64 kvm_timer_compute_delta(struct kvm_vcpu *vcpu,
>> return 0;
>> }
>>
>> +static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx)
>> +{
>> + return !(timer_ctx->cnt_ctl & ARCH_TIMER_CTRL_IT_MASK) &&
>> + (timer_ctx->cnt_ctl & ARCH_TIMER_CTRL_ENABLE);
>> +}
>> +
>> +/*
>> + * Returns minimal timer expiration time in ns among guest timers.
>> + * Note that it will return inf time if none of timers can fire.
>> + */
>> +static u64 kvm_timer_min_block(struct kvm_vcpu *vcpu)
>> +{
>> + u64 min_virt = ULLONG_MAX, min_phys = ULLONG_MAX;
>> + struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>> +
>> + if (kvm_timer_irq_can_fire(vtimer))
>> + min_virt = kvm_timer_compute_delta(vcpu, vtimer);
>> +
>> + if (kvm_timer_irq_can_fire(ptimer))
>> + min_phys = kvm_timer_compute_delta(vcpu, ptimer);
>> +
>> + WARN_ON((min_virt == ULLONG_MAX) && (min_phys == ULLONG_MAX));
>> +
>> + return min(min_virt, min_phys);
>> +}
>> +
>> static enum hrtimer_restart kvm_timer_expire(struct hrtimer *hrt)
>> {
>> struct arch_timer_cpu *timer;
>> @@ -144,7 +172,7 @@ static enum hrtimer_restart kvm_timer_expire(struct hrtimer *hrt)
>> * PoV (NTP on the host may have forced it to expire
>> * early). If we should have slept longer, restart it.
>> */
>> - ns = kvm_timer_compute_delta(vcpu, vcpu_vtimer(vcpu));
>> + ns = kvm_timer_min_block(vcpu);
>> if (unlikely(ns)) {
>> hrtimer_forward_now(hrt, ns_to_ktime(ns));
>> return HRTIMER_RESTART;
>> @@ -154,12 +182,6 @@ static enum hrtimer_restart kvm_timer_expire(struct hrtimer *hrt)
>> return HRTIMER_NORESTART;
>> }
>>
>> -static bool kvm_timer_irq_can_fire(struct arch_timer_context *timer_ctx)
>> -{
>> - return !(timer_ctx->cnt_ctl & ARCH_TIMER_CTRL_IT_MASK) &&
>> - (timer_ctx->cnt_ctl & ARCH_TIMER_CTRL_ENABLE);
>> -}
>> -
>> bool kvm_timer_should_fire(struct kvm_vcpu *vcpu,
>> struct arch_timer_context *timer_ctx)
>> {
>> @@ -191,6 +213,21 @@ static void kvm_timer_update_mapped_irq(struct kvm_vcpu *vcpu, bool new_level,
>> WARN_ON(ret);
>> }
>>
>> +static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,
>> + struct arch_timer_context *timer)
>> +{
>> + int ret;
>> +
>> + BUG_ON(!vgic_initialized(vcpu->kvm));
>> +
>> + timer->irq.level = new_level;
>> + trace_kvm_timer_update_irq(vcpu->vcpu_id, timer->irq.irq,
>> + timer->irq.level);
>> + ret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id, timer->irq.irq,
>> + timer->irq.level);
>> + WARN_ON(ret);
>> +}
>> +
>> /*
>> * Check if there was a change in the timer state (should we raise or lower
>> * the line level to the GIC).
>> @@ -198,6 +235,7 @@ static void kvm_timer_update_mapped_irq(struct kvm_vcpu *vcpu, bool new_level,
>> static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
>> {
>> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>>
>> /*
>> * If userspace modified the timer registers via SET_ONE_REG before
>> @@ -211,6 +249,10 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
>> if (kvm_timer_should_fire(vcpu, vtimer) != vtimer->irq.level)
>> kvm_timer_update_mapped_irq(vcpu, !vtimer->irq.level, vtimer);
>>
>> + /* The emulated EL1 physical timer irq is not mapped to hardware */
>> + if (kvm_timer_should_fire(vcpu, ptimer) != ptimer->irq.level)
>> + kvm_timer_update_irq(vcpu, !ptimer->irq.level, ptimer);
>> +
>> return 0;
>> }
>>
>> @@ -223,26 +265,32 @@ void kvm_timer_schedule(struct kvm_vcpu *vcpu)
>> {
>> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
>> struct arch_timer_context *vtimer = vcpu_vtimer(vcpu);
>> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>>
>> BUG_ON(timer_is_armed(timer));
>>
>> /*
>> - * No need to schedule a background timer if the guest timer has
>> + * No need to schedule a background timer if any guest timer has
>> * already expired, because kvm_vcpu_block will return before putting
>> * the thread to sleep.
>> */
>> - if (kvm_timer_should_fire(vcpu, vtimer))
>> + if (kvm_timer_should_fire(vcpu, vtimer) ||
>> + kvm_timer_should_fire(vcpu, ptimer))
>> return;
>>
>> /*
>> - * If the timer is not capable of raising interrupts (disabled or
>> + * If both timers are not capable of raising interrupts (disabled or
>> * masked), then there's no more work for us to do.
>> */
>> - if (!kvm_timer_irq_can_fire(vtimer))
>> + if (!kvm_timer_irq_can_fire(vtimer) &&
>> + !kvm_timer_irq_can_fire(ptimer))
>> return;
>>
>> - /* The timer has not yet expired, schedule a background timer */
>> - timer_arm(timer, kvm_timer_compute_delta(vcpu, vtimer));
>> + /*
>> + * The guest timers have not yet expired, schedule a background timer.
>> + * Pick smaller expiration time between phys and virt timer.
>> + */
>> + timer_arm(timer, kvm_timer_min_block(vcpu));
>> }
>>
>> void kvm_timer_unschedule(struct kvm_vcpu *vcpu)
>> --
>> 1.9.1
>>
>>
>
^ permalink raw reply
* [PATCH 1/2] ARM: dts: imx6: Specify 'anatop-enable-bit' where appropriate
From: Fabio Estevam @ 2017-01-10 17:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110163015.22444-1-andrew.smirnov@gmail.com>
On Tue, Jan 10, 2017 at 2:30 PM, Andrey Smirnov
<andrew.smirnov@gmail.com> wrote:
> ENABLE_LINREG bit is implemented by 3P0, 1P1 and 2P5 regulators on
> i.MX6. This property is present in similar code in Fresscale BSP and
> made its way upstream in imx6ul.dtsi, so this patch adds this property
> to the rest of i.MX6 family for completness.
Please see:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/patch/arch/arm/boot/dts/imx6ul.dtsi?id=27958ccdf29e9971732e02494b48be54b0691269
^ permalink raw reply
* [GIT PULL] Allwinner conversion to generic pin properties
From: Maxime Ripard @ 2017-01-10 17:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170110032120.GJ22261@localhost>
Hi Olof,
On Mon, Jan 09, 2017 at 07:21:20PM -0800, Olof Johansson wrote:
> Hi,
>
> On Mon, Dec 26, 2016 at 08:42:14AM +0100, Maxime Ripard wrote:
> > Hi Arnd, Olof,
> >
> > Like we discussed it last time, here is a pull request with the
> > convertion patches to the generic pin properties for 4.10.
> >
> > Thanks,
> > Maxime
> >
> > The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77:
> >
> > Linux 4.10-rc1 (2016-12-25 16:13:08 -0800)
> >
> > are available in the git repository at:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git tags/sunxi-generic-pinconf-for-4.10
> >
> > for you to fetch changes up to 1edcd36fcb48fe841bcc62eda36c105037d9583c:
> >
> > ARM: sunxi: Convert pinctrl nodes to generic bindings (2016-12-26 08:27:11 +0100)
> >
> > ----------------------------------------------------------------
> > Allwinner conversion to generic pin muxing properties
> >
> > Here is a list of patches that converts the current DT to the generic pin
> > control and muxing properties, now that the pinctrl driver supports it.
>
> I dropped the ball on this over the holidays for a few reasons, none of them
> good. Unfortunately we're at -rc3+ now, so I've queued this up in next/dt for
> v4.11 at this point. Hope that's alright.
It is, there's no rush.
> You might want to use your branch as a base for new DT contents for
> v4.11 in case it ends up being conflict-heavy, which would work fine at our
> end.
There might be one minor (and trivial) conflict with one of the fix
I'll send your way, but that pretty much it. All the other patches
were based on this one already.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170110/4ffabc83/attachment.sig>
^ permalink raw reply
* [RFC 8/8] KVM: arm/arm64: Emulate the EL1 phys timer register access
From: Jintack Lim @ 2017-01-10 17:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170109121612.GG4348@cbox>
On Mon, Jan 9, 2017 at 7:16 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Mon, Dec 26, 2016 at 12:12:06PM -0500, Jintack Lim wrote:
>> Emulate read and write operations to CNTP_TVAL, CNTP_CVAL and CNTP_CTL.
>> Now the VM is able to use the EL1 physical timer.
>>
>> Signed-off-by: Jintack Lim <jintack@cs.columbia.edu>
>> ---
>> arch/arm64/kvm/sys_regs.c | 35 ++++++++++++++++++++++++++++++++---
>> include/kvm/arm_arch_timer.h | 3 +++
>> virt/kvm/arm/arch_timer.c | 4 ++--
>> 3 files changed, 37 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index fd9e747..7cef94f 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -824,7 +824,15 @@ static bool access_cntp_tval(struct kvm_vcpu *vcpu,
>> struct sys_reg_params *p,
>> const struct sys_reg_desc *r)
>> {
>> - kvm_inject_undefined(vcpu);
>> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>> + cycle_t now = kvm_phys_timer_read();
>> +
>> + if (p->is_write) {
>> + ptimer->cnt_cval = p->regval + now;
>> + kvm_timer_emulate(vcpu, ptimer);
>
> Hmm, do we really need those calls here?
>
> I guess I'm a little confused about exactly what the kvm_timer_emulate()
> function is supposed to do, and it feels to me like these handlers
> should just record what the guest is asking the kernel to do and the
> logic of handling the additional timer should be moved into the run path
> as much as possible.
I think it's a design decision. As you suggested, it's simple to do
set up the background timer on entry to the VM, cancel it on exit, but
since that's on the critical path it may have some impact on the
performance, especially the world switch cost. To avoid
canceling/setting up timer every world switch, I choose to schedule
the physical timer here. I haven't compared the cost of the two
alternatives, though.
>
> Thanks,
> -Christoffer
>
>> + } else
>> + p->regval = ptimer->cnt_cval - now;
>> +
>> return true;
>> }
>>
>> @@ -832,7 +840,21 @@ static bool access_cntp_ctl(struct kvm_vcpu *vcpu,
>> struct sys_reg_params *p,
>> const struct sys_reg_desc *r)
>> {
>> - kvm_inject_undefined(vcpu);
>> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>> +
>> + if (p->is_write) {
>> + /* ISTATUS bit is read-only */
>> + ptimer->cnt_ctl = p->regval & ~ARCH_TIMER_CTRL_IT_STAT;
>> + kvm_timer_emulate(vcpu, ptimer);
>> + } else {
>> + cycle_t now = kvm_phys_timer_read();
>> +
>> + p->regval = ptimer->cnt_ctl;
>> + /* Set ISTATUS bit if it's expired */
>> + if (ptimer->cnt_cval <= now)
>> + p->regval |= ARCH_TIMER_CTRL_IT_STAT;
>> + }
>> +
>> return true;
>> }
>>
>> @@ -840,7 +862,14 @@ static bool access_cntp_cval(struct kvm_vcpu *vcpu,
>> struct sys_reg_params *p,
>> const struct sys_reg_desc *r)
>> {
>> - kvm_inject_undefined(vcpu);
>> + struct arch_timer_context *ptimer = vcpu_ptimer(vcpu);
>> +
>> + if (p->is_write) {
>> + ptimer->cnt_cval = p->regval;
>> + kvm_timer_emulate(vcpu, ptimer);
>> + } else
>> + p->regval = ptimer->cnt_cval;
>> +
>> return true;
>> }
>>
>> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
>> index 04ed9c1..776579b 100644
>> --- a/include/kvm/arm_arch_timer.h
>> +++ b/include/kvm/arm_arch_timer.h
>> @@ -75,6 +75,9 @@ bool kvm_timer_should_fire(struct kvm_vcpu *vcpu,
>> struct arch_timer_context *timer_ctx);
>> void kvm_timer_schedule(struct kvm_vcpu *vcpu);
>> void kvm_timer_unschedule(struct kvm_vcpu *vcpu);
>> +void kvm_timer_emulate(struct kvm_vcpu *vcpu, struct arch_timer_context *timer);
>> +
>> +cycle_t kvm_phys_timer_read(void);
>>
>> void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu);
>>
>> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
>> index be8d953..7a161f8 100644
>> --- a/virt/kvm/arm/arch_timer.c
>> +++ b/virt/kvm/arm/arch_timer.c
>> @@ -39,7 +39,7 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)
>> vcpu_vtimer(vcpu)->active_cleared_last = false;
>> }
>>
>> -static cycle_t kvm_phys_timer_read(void)
>> +cycle_t kvm_phys_timer_read(void)
>> {
>> return timecounter->cc->read(timecounter->cc);
>> }
>> @@ -258,7 +258,7 @@ static int kvm_timer_update_state(struct kvm_vcpu *vcpu)
>> * Schedule the background timer for the emulated timer. The background timer
>> * runs whenever vcpu is runnable and the timer is not expired.
>> */
>> -static void kvm_timer_emulate(struct kvm_vcpu *vcpu,
>> +void kvm_timer_emulate(struct kvm_vcpu *vcpu,
>> struct arch_timer_context *timer_ctx)
>> {
>> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
>> --
>> 1.9.1
>>
>>
>
^ permalink raw reply
* [PATCH] coresight: STM: Balance enable/disable
From: Mathieu Poirier @ 2017-01-10 17:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484047315-14407-1-git-send-email-suzuki.poulose@arm.com>
On Tue, Jan 10, 2017 at 11:21:55AM +0000, Suzuki K Poulose wrote:
> The stm is automatically enabled when an application sets the policy
> via ->link() call back by using coresight_enable(), which keeps the
> refcount of the current users of the STM. However, the unlink() callback
> issues stm_disable() directly, which leaves the STM turned off, without
> the coresight layer knowing about it. This prevents any further uses
> of the STM hardware as the coresight layer still thinks the STM is
> turned on and doesn't issue an stm_enable(). Even manually enabling
> the STM via sysfs can't really enable the hw.
>
> e.g,
>
> $ echo 1 > $CS_DEVS/$ETR/enable_sink
> $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/
> $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels
> $ echo 64 > $CS_DEVS/$source/traceid
> $ ./stm_app
> Sending 64000 byte blocks of pattern 0 at 0us intervals
> Success to map channel(32768~32783) to 0xffffa95fa000
> Sending on channel 32768
> $ dd if=/dev/$ETR of=~/trace.bin.1
> 597+1 records in
> 597+1 records out
> 305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s
> $ ./stm_app
> Sending 64000 byte blocks of pattern 0 at 0us intervals
> Success to map channel(32768~32783) to 0xffff7e9e2000
> Sending on channel 32768
> $ dd if=/dev/$ETR of=~/trace.bin.2
> 0+0 records in
> 0+0 records out
> 0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s
>
> Note that we don't get any data from the ETR for the second session.
>
> Also dmesg shows :
>
> [ 77.520458] coresight-tmc 20800000.etr: TMC-ETR enabled
> [ 77.537097] coresight-replicator etr_replicator at 20890000: REPLICATOR enabled
> [ 77.558828] coresight-replicator main_replicator at 208a0000: REPLICATOR enabled
> [ 77.581068] coresight-funnel 208c0000.main_funnel: FUNNEL inport 0 enabled
> [ 77.602217] coresight-tmc 20840000.etf: TMC-ETF enabled
> [ 77.618422] coresight-stm 20860000.stm: STM tracing enabled
> [ 139.554252] coresight-stm 20860000.stm: STM tracing disabled
> # End of first tracing session
> [ 146.351135] coresight-tmc 20800000.etr: TMC read start
> [ 146.514486] coresight-tmc 20800000.etr: TMC read end
> # Note that the STM is not turned on via stm_generic_link()->coresight_enable()
> # and hence none of the components are turned on.
> [ 152.479080] coresight-tmc 20800000.etr: TMC read start
> [ 152.542632] coresight-tmc 20800000.etr: TMC read end
>
> This patch balances the unlink operation by using the coresight_disable(),
> keeping the coresight layer in sync with the hardware state.
>
> Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM component")
> Cc: Pratik Patel <pratikp@codeaurora.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Chunyan Zhang <zhang.chunyan@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable at vger.kernel.org # 4.7+
> Reported-by: Robert Walker <robert.walker@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-stm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index 3524452..57b7330 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
> if (!drvdata || !drvdata->csdev)
> return;
>
> - stm_disable(drvdata->csdev, NULL);
> + coresight_disable(drvdata->csdev);
This looks valid to me.
Chunyan, any reason to use stm_disable() directly rather than calling it as part
of the device OPS in coresight_disable()?
Thanks,
Mathieu
> }
>
> static phys_addr_t
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v2 0/3] arm64: dts: exynos: Fix DTC warnings for Exynos boards
From: Javier Martinez Canillas @ 2017-01-10 17:38 UTC (permalink / raw)
To: linux-arm-kernel
Hello Krzysztof,
This trivial series contains fixes for DTC warnings caused by mismatches
between unit names and reg properties in device tree nodes.
The patches shouldn't cause a functional change but have been just build
tested. I compared the generated DTB though to make sure that only these
nodes changed.
Best regards,
Javier
Changes since v1:
- Fix subject line since I forgot the "exynos" prefix.
Javier Martinez Canillas (3):
arm64: dts: exynos: Add missing unit name to Exynos7 SoC node
arm64: dts: exynos: Add missing unit name to Exynos5433 SoC node
arm64: dts: exynos: Remove unneeded unit names in Exynos5433 nodes
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 8 ++++----
arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v2 1/3] arm64: dts: exynos: Add missing unit name to Exynos7 SoC node
From: Javier Martinez Canillas @ 2017-01-10 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484069912-6534-1-git-send-email-javier@osg.samsung.com>
This patch fixes the following DTC warning about a mismatch
between a device node reg property and its unit name:
Node /soc has a reg or ranges property, but no unit name
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
arch/arm64/boot/dts/exynos/exynos7.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi b/arch/arm64/boot/dts/exynos/exynos7.dtsi
index 80aa60e38237..0d2fedc6ac2f 100644
--- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
@@ -69,7 +69,7 @@
method = "smc";
};
- soc: soc {
+ soc: soc at 0 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/3] arm64: dts: exynos: Add missing unit name to Exynos5433 SoC node
From: Javier Martinez Canillas @ 2017-01-10 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484069912-6534-1-git-send-email-javier@osg.samsung.com>
This patch fixes the following DTC warning about a mismatch
between a device node reg property and its unit name:
Node /soc has a reg or ranges property, but no unit name
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 68f764e5851c..3695ddaf2e04 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -241,7 +241,7 @@
mask = <0x1>;
};
- soc: soc {
+ soc: soc at 0 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 3/3] arm64: dts: exynos: Remove unneeded unit names in Exynos5433 nodes
From: Javier Martinez Canillas @ 2017-01-10 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484069912-6534-1-git-send-email-javier@osg.samsung.com>
The "samsung,exynos5433-mipi-video-phy" and "samsung,exynos5250-dwusb3"
DT bindings don't specify a reg property for these nodes, so having a
unit name leads to the following DTC warnings:
Node /soc/video-phy at 105c0710 has a unit name, but no reg property
Node /soc/usb at 15400000 has a unit name, but no reg property
Node /soc/usb at 15a00000 has a unit name, but no reg property
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 3695ddaf2e04..17e5dafd392c 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -706,7 +706,7 @@
interrupts = <GIC_PPI 9 0xf04>;
};
- mipi_phy: video-phy at 105c0710 {
+ mipi_phy: video-phy {
compatible = "samsung,exynos5433-mipi-video-phy";
#phy-cells = <1>;
samsung,pmu-syscon = <&pmu_system_controller>;
@@ -1285,7 +1285,7 @@
status = "disabled";
};
- usbdrd30: usb at 15400000 {
+ usbdrd30: usb-0 {
compatible = "samsung,exynos5250-dwusb3";
clocks = <&cmu_fsys CLK_ACLK_USBDRD30>,
<&cmu_fsys CLK_SCLK_USBDRD30>;
@@ -1332,7 +1332,7 @@
status = "disabled";
};
- usbhost30: usb at 15a00000 {
+ usbhost30: usb-1 {
compatible = "samsung,exynos5250-dwusb3";
clocks = <&cmu_fsys CLK_ACLK_USBHOST30>,
<&cmu_fsys CLK_SCLK_USBHOST30>;
--
2.7.4
^ permalink raw reply related
* [PATCH v3 00/10] perf: arm64: Support for Hisilicon SoC Hardware event counters
From: Mark Rutland @ 2017-01-10 17:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1483339672-23778-1-git-send-email-anurup.m@huawei.com>
Hi,
On Mon, Jan 02, 2017 at 01:47:52AM -0500, Anurup M wrote:
> ToDo:
> 1) The counter overflow handling is currently unsupported in this
> patch series.
>From a quick scan of the patches, I see mention of an interrupt in a
comment the driver, but there's noething in the DT binding.
Is there an overflow interrupt at all?
Or do you need to implement polling to avoid overflow?
This is a prerequisite for merging the driver.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v3 02/10] dt-bindings: hisi: Add Hisilicon HiP05/06/07 Djtag dts bindings
From: Mark Rutland @ 2017-01-10 17:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <586DD28E.5050500@gmail.com>
On Thu, Jan 05, 2017 at 10:28:54AM +0530, Anurup M wrote:
>
>
> On Wednesday 04 January 2017 04:26 AM, Rob Herring wrote:
> >On Mon, Jan 02, 2017 at 01:49:03AM -0500, Anurup M wrote:
> >>From: Tan Xiaojun <tanxiaojun@huawei.com>
> >>
> >>Add Hisilicon HiP05/06/07 Djtag dts bindings for CPU and IO Die
> >>
> >>Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
> >>Signed-off-by: Anurup M <anurup.m@huawei.com>
> >>---
> >> .../devicetree/bindings/arm/hisilicon/djtag.txt | 41 ++++++++++++++++++++++
> >> 1 file changed, 41 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/djtag.txt
> >>
> >>diff --git a/Documentation/devicetree/bindings/arm/hisilicon/djtag.txt b/Documentation/devicetree/bindings/arm/hisilicon/djtag.txt
> >>new file mode 100644
> >>index 0000000..bbe8b45
> >>--- /dev/null
> >>+++ b/Documentation/devicetree/bindings/arm/hisilicon/djtag.txt
> >>@@ -0,0 +1,41 @@
> >>+The Hisilicon Djtag is an independent component which connects with some other
> >>+components in the SoC by Debug Bus. The djtag is available in CPU and IO dies
> >>+in the chip. The djtag controls access to connecting modules of CPU and IO
> >>+dies.
> >>+The various connecting components in CPU die (like L3 cache, L3 cache PMU etc.)
> >>+are accessed by djtag during real time debugging. In IO die there are connecting
> >>+components like RSA. These components appear as devices attached to djtag bus.
> >>+
> >>+Hisilicon HiP05/06/07 djtag for CPU and IO die
> >>+Required properties:
> >>+ - compatible : The value should be as follows
> >>+ (a) "hisilicon,hip05-djtag-v1" for CPU and IO die which use v1 hw in
> >>+ HiP05 chipset.
> >You don't need to distinguish the CPU and IO blocks?
>
> The CPU and IO djtag nodes will have different base address(in reg
> property).
> There is no difference in handling of CPU and IO dies in the djtag driver.
> So there is currently no need to distinguish them.
It may still make sense to distinuguish them, even if the current djtag
driver can handle them the same. Presumably clients of the djtag driver
will poke CPU and IO djtag units differently.
Thanks,
Mark.
^ permalink raw reply
* [PATCH] usb: dwc3-exynos fix unspecified suspend clk error handling
From: vivek.gautam at codeaurora.org @ 2017-01-10 17:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5156028.4V9iLD3Qkj@amdc3058>
On 2017-01-10 22:39, Bartlomiej Zolnierkiewicz wrote:
> Hi,
>
> On Tuesday, January 10, 2017 09:28:52 AM Shuah Khan wrote:
>> On 01/10/2017 09:05 AM, Bartlomiej Zolnierkiewicz wrote:
>> >
>> > Hi,
>> >
>> > On Tuesday, January 10, 2017 07:36:35 AM Shuah Khan wrote:
>> >> On 01/10/2017 07:16 AM, Shuah Khan wrote:
>> >>> On 01/10/2017 05:05 AM, Bartlomiej Zolnierkiewicz wrote:
>> >>>>
>> >>>> Hi,
>> >>>>
>> >>>> On Monday, January 09, 2017 07:21:31 PM Shuah Khan wrote:
>> >>>>> Fix dwc3_exynos_probe() to call clk_prepare_enable() only when suspend
>> >>>>> clock is specified. Call clk_disable_unprepare() from remove and probe
>> >>>>> error path only when susp_clk has been set from remove and probe error
>> >>>>> paths.
>> >>>>
>> >>>> It is legal to call clk_prepare_enable() and clk_disable_unprepare()
>> >>>> for NULL clock. Also your patch changes susp_clk handling while
>> >>>> leaves axius_clk handling (which also can be NULL) untouched.
>> >>>>
>> >>>> Do you actually see some runtime problem with the current code?
>> >>>>
>> >>>> If not then the patch should probably be dropped.
>> >>>>
>> >>>> Best regards,
>> >>>> --
>> >>>> Bartlomiej Zolnierkiewicz
>> >>>> Samsung R&D Institute Poland
>> >>>> Samsung Electronics
>> >>>
>> >>> Hi Bartlomiej,
>> >>>
>> >>> I am seeing the "no suspend clk specified" message in dmesg.
>> >>> After that it sets the exynos->susp_clk = NULL and starts
>> >>> calling clk_prepare_enable(exynos->susp_clk);
>> >>>
>> >>> That can't be good. If you see the logic right above this
>> >>> one for exynos->clk, it returns error and fails the probe.
>> >>> This this case it doesn't, but tries to use null susp_clk.
>> >
>> > exynos->susp_clk is optional, exynos->clk is not.
>>
>> Right. That is clear since we don't fail the probe.
>>
>> >
>> >>> I believe this patch is necessary.
>> >>
>> >> Let me clarify this a bit further. Since we already know
>> >> susp_clk is null, with this patch we can avoid extra calls
>> >> to clk_prepare_enable() and clk_disable_unprepare().
>> >>
>> >> One can say, it also adds extra checks, hence I will let you
>> >> decide one way or the other. :)
Hi Shuah Khan,
Like Bartlomiej mentioned below, it is completely fair to call
clk_prepare_enable() with NULL clocks. That's how most of the
optional clocks are handled in the kernel. So, i don't think
there's any need of adding an additional check for the
'exynos->susp_clk'.
The 'exynos->clk' is the main IP clock that is mandatory, and hence
the probe fails in case that is not present.
>> >
>> > I would prefer to leave the things as they are currently.
>> >
>> > The code in question is not performance sensitive so extra
>> > calls are not a problem. No extra checks means less code.
>> >
>> > Also the current code seems to be more in line with the rest
>> > of the kernel.
>>
>> What functionality is missing without the suspend clock? Would
>> it make sense to change the info. message to include what it
>> means. At the moment it doesn't anything more than "no suspend
>> clock" which is a very cryptic user visible message. It would be
>> helpful for it to also include what functionality is impacted.
>
> Well, all I know is what the original commit descriptions says and
> that the commit itself comes from patchset adding Exynos7 USB 3.0
> support [1]:
>
> commit 72d996fc7a01c2e4d581a15db7d001e2799ffb29
> Author: Vivek Gautam <gautam.vivek@samsung.com>
> Date: Fri Nov 21 19:05:46 2014 +0530
>
> usb: dwc3: exynos: Add provision for suspend clock
>
> DWC3 controller on Exynos SoC series have separate control for
> suspend clock which replaces pipe3_rx_pclk as clock source to
> a small part of DWC3 core that operates when SS PHY is in its
> lowest power state (P3) in states SS.disabled and U3.
>
> Suggested-by: Anton Tikhomirov <av.tikhomirov@samsung.com>
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
>
Hi Bartlomiej,
> Anton's & Vivek's Samsung email addresses are no longer valid but
> I added current Vivek's email address to Cc:.
Thanks for adding me to the thread.
>
> BTW What is interesting is that the Exynos7 dts patch [2] has never
> made it into upstream for some reason. In the meantime however
> Exynos5433 (similar to Exynos7 to some degree) became the user of
> susp_clk.
You are right. The exynos7 device tree patches could not make it to
upstream for some reasons. I think there are few guys looking at USB
for Exynos.
If there's something needed on Exynos7 USB side, i have added
Pankaj Dubey <pankaj.dubey@samsung.com> to this thread.
Hi Pankaj,
I am adding you to please help us with any future requirements
for exynos USB in the upstream.
Thanks!
>
> [1] https://lkml.org/lkml/2014/11/21/247
> [2] https://patchwork.kernel.org/patch/5355161/
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
[snip]
Best Regards
Vivek
^ permalink raw reply
* [PATCH V6 05/10] acpi: apei: handle SEA notification type for ARMv8
From: Baicar, Tyler @ 2017-01-10 17:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <586F74DB.3020107@arm.com>
Hello James,
On 1/6/2017 3:43 AM, James Morse wrote:
> On 05/01/17 22:31, Baicar, Tyler wrote:
>> On 12/20/2016 8:29 AM, James Morse wrote:
>>> On 07/12/16 21:48, Tyler Baicar wrote:
>>>> ARM APEI extension proposal added SEA (Synchrounous External
>>>> Abort) notification type for ARMv8.
>>>> Add a new GHES error source handling function for SEA. If an error
>>>> source's notification type is SEA, then this function can be registered
>>>> into the SEA exception handler. That way GHES will parse and report
>>>> SEA exceptions when they occur.
>>>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>>>> index 2acbc60..66ab3fd 100644
>>>> --- a/drivers/acpi/apei/ghes.c
>>>> +++ b/drivers/acpi/apei/ghes.c
>>>> @@ -767,6 +771,62 @@ static struct notifier_block ghes_notifier_sci = {
>>>> .notifier_call = ghes_notify_sci,
>>>> };
>>>> +#ifdef CONFIG_HAVE_ACPI_APEI_SEA
>>>> +static LIST_HEAD(ghes_sea);
>>>> +
>>>> +static int ghes_notify_sea(struct notifier_block *this,
>>>> + unsigned long event, void *data)
>>>> +{
>>>> + struct ghes *ghes;
>>>> + int ret = NOTIFY_DONE;
>>>> +
>>>> + rcu_read_lock();
>>>> + list_for_each_entry_rcu(ghes, &ghes_sea, list) {
>>>> + if (!ghes_proc(ghes))
>>>> + ret = NOTIFY_OK;
>>>> + }
>>>> + rcu_read_unlock();
>>>> +
>>>> + return ret;
>>>> +}
>>> What stops this from being re-entrant?
>>>
>>> ghes_copy_tofrom_phs() takes the ghes_ioremap_lock_irq spinlock, but there is
>>> nothing to stop a subsequent instruction fetch or memory access causing another
>>> (maybe different) Synchronous External Abort which deadlocks trying to take the
>>> same lock.
>>>
>>> ghes_notify_sea() looks to be based on ghes_notify_sci(), which (if I've found
>>> the right part of the ACPI spec) is a level-low interrupt. spin_lock_irqsave()
>>> would mask interrupts so there is no risk of a different notification firing on
>>> the same CPU, (it looks like they are almost all ultimately an irq).
>>>
>>> NMI is the odd one out because its not maskable like this, but ghes_notify_nmi()
>>> has:
>>>> if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
>>>> return ret;
>>> To ensure there is only ever one thread poking around in this code.
>>>
>>> What happens if a system describes two GHES sources, one using an irq the other
>>> SEA? The SEA error can interrupt the irq error while its holding the above lock.
>>> I guess this is also why all the NMI code in that file is separate.
>
>> Let me see if I'm following you right :)
>> I should use spin_lock_irqsave() in ghes_notify_sea() to avoid ghes_notify_sci()
>> from
>> interrupting this process and potentially causing the deadlock?
> This way round you are already safe: The CPU masks interrupts when it takes the
> exception, they should still be masked by the time we get in here...
>
> The other way round is a lot more fun!
>
> What happens if APEI is processing some error record that was notified via an
> interrupt, and then takes the Synchronous External Abort, and ends up back in
> this code? Masking interrupts doesn't stop the external-abort, and trying to
> take the ghes_ioremap_lock_irq will deadlock.
>
> What happens if we interrupt printk() holding all its locks is another thing I
> haven't worked out yet.
>
>
>> This race condition does seem valid. We are using the same acknowledgment for
>> all our
>> HEST table entries, so our firmware will not populate more than one entry at a
>> time. That
>> gets us around this race condition.
> Ah, so your firmware will wait for the interrupt-signalled error to be finished
> before it triggers the Synchronous External Abort. I think this would still be a
> linux bug if the firmware didn't do this.
>
> x86 could have done the same with NMI notifications, but we have all this 'if
> (in_nmi)' to allow interrupts-masked GHES handling to be interrupted.
>
> What do you think to re-using the 'if (in_nmi)' code for SEA? We can argue that
> SEA is NMI-like in that it can't be masked, and it interrupts code that had
> interrupts masked. It 'should' be as simple as putting 'HAVE_NMI' in arm64's
> Kconfig, and wrapping the atomic notifier call with nmi_enter()/nmi_exit() from
> linux/hardirq.h. (...famous last words...)
>
> This probably answers my printk() questions too, but I need to look into it some
> more.
Thanks for the detailed description! I looked through this and it seems
like re-using the NMI code should work. I'll add the use of the in_nmi
code in the next patchset.
Thanks,
Tyler
--
Qualcomm Datacenter Technologies, Inc. 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
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