* [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes
From: Fabio Estevam @ 2017-01-19 14:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119141335.kpbrbpbwaovnijjv@pengutronix.de>
Hi Uwe,
On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> Would it be nice to add a comment about why this was added? Something to
> prevent a cleanup like "remove empty nodes and invalid memory
> configurations".
Do you mean something like this?
/* "chosen" and "memory" nodes are mandatory */
chosen {};
memory { device_type = "memory"; reg = <0 0>; };
^ permalink raw reply
* [PATCH v4] iommu/arm-smmu: Support for Extended Stream ID (16 bit)
From: Aleksey Makarov @ 2017-01-19 14:36 UTC (permalink / raw)
To: linux-arm-kernel
It is the time we have the real 16-bit Stream ID user, which is the
ThunderX. Its IO topology uses 1:1 map for Requester ID to Stream ID
translation for each root complex which allows to get full 16-bit
Stream ID. Firmware assigns bus IDs that are greater than 128 (0x80)
to some buses under PEM (external PCIe interface). Eventually SMMU
drops devices on that buses because their Stream ID is out of range:
pci 0006:90:00.0: stream ID 0x9000 out of range for SMMU (0x7fff)
To fix above issue enable the Extended Stream ID optional feature
when available.
Reviewed-by: Tomasz Nowicki <tomasz.nowicki@caviumnetworks.com>
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
Tested-by: Tomasz Nowicki <tomasz.nowicki@caviumnetworks.com>
---
v4:
change the commit message:
- explain the reason why the warning happens (Tomasz Nowicki)
- add 'Reviewed-by' and 'Tested-by' from Tomasz Nowicki
v3:
https://lkml.kernel.org/r/20170117151431.17085-1-aleksey.makarov at linaro.org
- keep formatting of the comment
- fix printk message after the deleted chunk. "[Do] not print a mask
here, since it's not overly interesting in itself, and add_device will
still show the offending mask in full if it ever actually matters (as in
the commit message)." (Robin Murphy)
v2:
https://lkml.kernel.org/r/20170116141111.29444-1-aleksey.makarov at linaro.org
- remove unnecessary parentheses (Robin Murphy)
- refactor testing SMR fields to after setting sCR0 as theirs width
depends on sCR0_EXIDENABLE (Robin Murphy)
v1 (rfc):
https://lkml.kernel.org/r/20170110115755.19102-1-aleksey.makarov at linaro.org
drivers/iommu/arm-smmu.c | 69 +++++++++++++++++++++++++++++++++---------------
1 file changed, 48 insertions(+), 21 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 13d26009b8e0..861cc135722f 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -24,6 +24,7 @@
* - v7/v8 long-descriptor format
* - Non-secure access to the SMMU
* - Context fault reporting
+ * - Extended Stream ID (16 bit)
*/
#define pr_fmt(fmt) "arm-smmu: " fmt
@@ -87,6 +88,7 @@
#define sCR0_CLIENTPD (1 << 0)
#define sCR0_GFRE (1 << 1)
#define sCR0_GFIE (1 << 2)
+#define sCR0_EXIDENABLE (1 << 3)
#define sCR0_GCFGFRE (1 << 4)
#define sCR0_GCFGFIE (1 << 5)
#define sCR0_USFCFG (1 << 10)
@@ -126,6 +128,7 @@
#define ID0_NUMIRPT_MASK 0xff
#define ID0_NUMSIDB_SHIFT 9
#define ID0_NUMSIDB_MASK 0xf
+#define ID0_EXIDS (1 << 8)
#define ID0_NUMSMRG_SHIFT 0
#define ID0_NUMSMRG_MASK 0xff
@@ -169,6 +172,7 @@
#define ARM_SMMU_GR0_S2CR(n) (0xc00 + ((n) << 2))
#define S2CR_CBNDX_SHIFT 0
#define S2CR_CBNDX_MASK 0xff
+#define S2CR_EXIDVALID (1 << 10)
#define S2CR_TYPE_SHIFT 16
#define S2CR_TYPE_MASK 0x3
enum arm_smmu_s2cr_type {
@@ -354,6 +358,7 @@ struct arm_smmu_device {
#define ARM_SMMU_FEAT_FMT_AARCH64_64K (1 << 9)
#define ARM_SMMU_FEAT_FMT_AARCH32_L (1 << 10)
#define ARM_SMMU_FEAT_FMT_AARCH32_S (1 << 11)
+#define ARM_SMMU_FEAT_EXIDS (1 << 12)
u32 features;
#define ARM_SMMU_OPT_SECURE_CFG_ACCESS (1 << 0)
@@ -1051,7 +1056,7 @@ static void arm_smmu_write_smr(struct arm_smmu_device *smmu, int idx)
struct arm_smmu_smr *smr = smmu->smrs + idx;
u32 reg = smr->id << SMR_ID_SHIFT | smr->mask << SMR_MASK_SHIFT;
- if (smr->valid)
+ if (!(smmu->features & ARM_SMMU_FEAT_EXIDS) && smr->valid)
reg |= SMR_VALID;
writel_relaxed(reg, ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_SMR(idx));
}
@@ -1063,6 +1068,9 @@ static void arm_smmu_write_s2cr(struct arm_smmu_device *smmu, int idx)
(s2cr->cbndx & S2CR_CBNDX_MASK) << S2CR_CBNDX_SHIFT |
(s2cr->privcfg & S2CR_PRIVCFG_MASK) << S2CR_PRIVCFG_SHIFT;
+ if (smmu->features & ARM_SMMU_FEAT_EXIDS && smmu->smrs &&
+ smmu->smrs[idx].valid)
+ reg |= S2CR_EXIDVALID;
writel_relaxed(reg, ARM_SMMU_GR0(smmu) + ARM_SMMU_GR0_S2CR(idx));
}
@@ -1073,6 +1081,34 @@ static void arm_smmu_write_sme(struct arm_smmu_device *smmu, int idx)
arm_smmu_write_smr(smmu, idx);
}
+/*
+ * The width of SMR's mask field depends on sCR0_EXIDENABLE, so this function
+ * should be called after sCR0 is written.
+ */
+static void arm_smmu_test_smr_masks(struct arm_smmu_device *smmu)
+{
+ void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
+ u32 smr;
+
+ if (!smmu->smrs)
+ return;
+
+ /*
+ * SMR.ID bits may not be preserved if the corresponding MASK
+ * bits are set, so check each one separately. We can reject
+ * masters later if they try to claim IDs outside these masks.
+ */
+ smr = smmu->streamid_mask << SMR_ID_SHIFT;
+ writel_relaxed(smr, gr0_base + ARM_SMMU_GR0_SMR(0));
+ smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
+ smmu->streamid_mask = smr >> SMR_ID_SHIFT;
+
+ smr = smmu->streamid_mask << SMR_MASK_SHIFT;
+ writel_relaxed(smr, gr0_base + ARM_SMMU_GR0_SMR(0));
+ smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
+ smmu->smr_mask_mask = smr >> SMR_MASK_SHIFT;
+}
+
static int arm_smmu_find_sme(struct arm_smmu_device *smmu, u16 id, u16 mask)
{
struct arm_smmu_smr *smrs = smmu->smrs;
@@ -1674,6 +1710,9 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
if (smmu->features & ARM_SMMU_FEAT_VMID16)
reg |= sCR0_VMID16EN;
+ if (smmu->features & ARM_SMMU_FEAT_EXIDS)
+ reg |= sCR0_EXIDENABLE;
+
/* Push the button */
__arm_smmu_tlb_sync(smmu);
writel(reg, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
@@ -1761,11 +1800,14 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
"\t(IDR0.CTTW overridden by FW configuration)\n");
/* Max. number of entries we have for stream matching/indexing */
- size = 1 << ((id >> ID0_NUMSIDB_SHIFT) & ID0_NUMSIDB_MASK);
+ if (smmu->version == ARM_SMMU_V2 && id & ID0_EXIDS) {
+ smmu->features |= ARM_SMMU_FEAT_EXIDS;
+ size = 1 << 16;
+ } else {
+ size = 1 << ((id >> ID0_NUMSIDB_SHIFT) & ID0_NUMSIDB_MASK);
+ }
smmu->streamid_mask = size - 1;
if (id & ID0_SMS) {
- u32 smr;
-
smmu->features |= ARM_SMMU_FEAT_STREAM_MATCH;
size = (id >> ID0_NUMSMRG_SHIFT) & ID0_NUMSMRG_MASK;
if (size == 0) {
@@ -1774,21 +1816,6 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
return -ENODEV;
}
- /*
- * SMR.ID bits may not be preserved if the corresponding MASK
- * bits are set, so check each one separately. We can reject
- * masters later if they try to claim IDs outside these masks.
- */
- smr = smmu->streamid_mask << SMR_ID_SHIFT;
- writel_relaxed(smr, gr0_base + ARM_SMMU_GR0_SMR(0));
- smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
- smmu->streamid_mask = smr >> SMR_ID_SHIFT;
-
- smr = smmu->streamid_mask << SMR_MASK_SHIFT;
- writel_relaxed(smr, gr0_base + ARM_SMMU_GR0_SMR(0));
- smr = readl_relaxed(gr0_base + ARM_SMMU_GR0_SMR(0));
- smmu->smr_mask_mask = smr >> SMR_MASK_SHIFT;
-
/* Zero-initialised to mark as invalid */
smmu->smrs = devm_kcalloc(smmu->dev, size, sizeof(*smmu->smrs),
GFP_KERNEL);
@@ -1796,8 +1823,7 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
return -ENOMEM;
dev_notice(smmu->dev,
- "\tstream matching with %lu register groups, mask 0x%x",
- size, smmu->smr_mask_mask);
+ "\tstream matching with %lu register groups", size);
}
/* s2cr->type == 0 means translation, so initialise explicitly */
smmu->s2crs = devm_kmalloc_array(smmu->dev, size, sizeof(*smmu->s2crs),
@@ -2120,6 +2146,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
iommu_register_instance(dev->fwnode, &arm_smmu_ops);
platform_set_drvdata(pdev, smmu);
arm_smmu_device_reset(smmu);
+ arm_smmu_test_smr_masks(smmu);
/* Oh, for a proper bus abstraction */
if (!iommu_present(&platform_bus_type))
--
2.11.0
^ permalink raw reply related
* [PATCH] arm64/cpufeatures: Enforce inline/const properties of cpus_have_const_cap
From: Will Deacon @ 2017-01-19 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484740725-24776-1-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 18, 2017 at 11:58:45AM +0000, Marc Zyngier wrote:
> Despite being flagged "inline", cpus_have_const_cap may end-up being
> placed out of line if the compiler decides so. This would be unfortunate,
> as we want to be able to use this function in HYP, where we need to
> be 100% sure of what is mapped there. __always_inline seems to be a
> better choice given the constraint.
>
> Also, be a lot tougher on non-const or out-of-range capability values
> (a non-const cap value shouldn't be used here, and the semantic of an
> OOR value is at best ill defined). In those two case, BUILD_BUG_ON is
> what you get.
>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/include/asm/cpufeature.h | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index b4989df..4710469 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -105,10 +105,11 @@ static inline bool cpu_have_feature(unsigned int num)
> }
>
> /* System capability check for constant caps */
> -static inline bool cpus_have_const_cap(int num)
> +static __always_inline bool cpus_have_const_cap(int num)
> {
> - if (num >= ARM64_NCAPS)
> - return false;
> + BUILD_BUG_ON(!__builtin_constant_p(num));
> + BUILD_BUG_ON(num >= ARM64_NCAPS);
This gives different behaviour to cpus_have_const_cap when compared to
cpus_have_cap, which I really don't like. What is the current behaviour
if you pass a non-constant num parameter? Does the kernel actually build?
Maybe it's best to spin a separate patch that makes cpus_have_cap and
cpus_have_const_cap both use __always_inline, then we can debate the merit
of the BUILD_BUG_ONs separately.
Will
^ permalink raw reply
* [PATCH 0/7] Fix issues and factorize arm/arm64 capacity information code
From: Juri Lelli @ 2017-01-19 14:37 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
arm and arm64 topology.c share a lot of code related to parsing of capacity
information. This set of patches proposes a solution (based on Will's,
Catalin's and Mark's off-line suggestions) to move such common code in a single
place: drivers/base/arch_topology.c (by creating such file and conditionally
compiling it for arm and arm64 only).
First 5 patches are actually fixes for the current code.
Patch 6 is the actual refactoring.
Last patch removes one of the extern symbols by changing a bit the now common
code. We still remain with some other externs, which are not nice. Moving them
in some header file solves the issue, should I just create a new include/
linux/arch_topology.h file and move them there?
The set is based on top of linux/master (4.10-rc4 fb1d8e0e2c50) and it is also
available from:
git://linux-arm.org/linux-jl.git upstream/default_caps_factorize
Best,
- Juri
Juri Lelli (7):
Documentation: arm: fix wrong reference number in DT definition
Documentation/ABI: add information about cpu_capacity
arm: fix return value of parse_cpu_capacity
arm: remove wrong CONFIG_PROC_SYSCTL ifdef
arm64: remove wrong CONFIG_PROC_SYSCTL ifdef
arm, arm64: factorize common cpu capacity default code
arm,arm64,drivers: reduce scope of cap_parsing_failed
Documentation/ABI/testing/sysfs-devices-system-cpu | 7 +
Documentation/devicetree/bindings/arm/cpus.txt | 4 +-
arch/arm/Kconfig | 1 +
arch/arm/kernel/topology.c | 216 +------------------
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/topology.c | 218 +------------------
drivers/base/Kconfig | 8 +
drivers/base/Makefile | 1 +
drivers/base/arch_topology.c | 240 +++++++++++++++++++++
9 files changed, 269 insertions(+), 427 deletions(-)
create mode 100644 drivers/base/arch_topology.c
--
2.10.0
^ permalink raw reply
* [PATCH 1/7] Documentation: arm: fix wrong reference number in DT definition
From: Juri Lelli @ 2017-01-19 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119143757.14537-1-juri.lelli@arm.com>
Reference to cpu capacity binding has a wrong number. Fix it.
Reported-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
Documentation/devicetree/bindings/arm/cpus.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index a1bcfeed5f24..c27376a27a92 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -246,7 +246,7 @@ nodes to be present and contain the properties described below.
Usage: Optional
Value type: <u32>
Definition:
- # u32 value representing CPU capacity [3] in
+ # u32 value representing CPU capacity [4] in
DMIPS/MHz, relative to highest capacity-dmips-mhz
in the system.
@@ -473,5 +473,5 @@ cpus {
[2] arm/msm/qcom,kpss-acc.txt
[3] ARM Linux kernel documentation - idle states bindings
Documentation/devicetree/bindings/arm/idle-states.txt
-[3] ARM Linux kernel documentation - cpu capacity bindings
+[4] ARM Linux kernel documentation - cpu capacity bindings
Documentation/devicetree/bindings/arm/cpu-capacity.txt
--
2.10.0
^ permalink raw reply related
* [PATCH 2/7] Documentation/ABI: add information about cpu_capacity
From: Juri Lelli @ 2017-01-19 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119143757.14537-1-juri.lelli@arm.com>
/sys/devices/system/cpu/cpu#/cpu_capacity describe information about
CPUs heterogeneity (ref. to Documentation/devicetree/bindings/arm/
cpu-capacity.txt).
Add such description.
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
Documentation/ABI/testing/sysfs-devices-system-cpu | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index 2a4a423d08e0..f3d5817c4ef0 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -366,3 +366,10 @@ Contact: Linux ARM Kernel Mailing list <linux-arm-kernel@lists.infradead.org>
Description: AArch64 CPU registers
'identification' directory exposes the CPU ID registers for
identifying model and revision of the CPU.
+
+What: /sys/devices/system/cpu/cpu#/cpu_capacity
+Date: December 2016
+Contact: Linux kernel mailing list <linux-kernel@vger.kernel.org>
+Description: information about CPUs heterogeneity.
+
+ cpu_capacity: capacity of cpu#.
--
2.10.0
^ permalink raw reply related
* [PATCH 3/7] arm: fix return value of parse_cpu_capacity
From: Juri Lelli @ 2017-01-19 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119143757.14537-1-juri.lelli@arm.com>
parse_cpu_capacity() has to return 0 on failure, but it currently returns
1 instead if raw_capacity kcalloc failed.
Fix it by removing the negation of the return value.
Cc: Russell King <linux@arm.linux.org.uk>
Reported-by: Morten Rasmussen <morten.rasmussen@arm.com>
Fixes: 06073ee26775 ('ARM: 8621/3: parse cpu capacity-dmips-mhz from DT')
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
arch/arm/kernel/topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index ebf47d91b804..b439f7fff86b 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -165,7 +165,7 @@ static int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
if (!raw_capacity) {
pr_err("cpu_capacity: failed to allocate memory for raw capacities\n");
cap_parsing_failed = true;
- return !ret;
+ return ret;
}
}
capacity_scale = max(cpu_capacity, capacity_scale);
--
2.10.0
^ permalink raw reply related
* [PATCH 4/7] arm: remove wrong CONFIG_PROC_SYSCTL ifdef
From: Juri Lelli @ 2017-01-19 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119143757.14537-1-juri.lelli@arm.com>
The sysfs cpu_capacity entry for each CPU has nothing to do with
PROC_FS, nor it's in /proc/sys path.
Remove such ifdef.
Cc: Russell King <linux@arm.linux.org.uk>
Reported-and-suggested-by: Sudeep Holla <sudeep.holla@arm.com>
Fixes: 7e5930aaef5d ('ARM: 8622/3: add sysfs cpu_capacity attribute')
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
arch/arm/kernel/topology.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index b439f7fff86b..c760a321935b 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -56,7 +56,6 @@ static void set_capacity_scale(unsigned int cpu, unsigned long capacity)
per_cpu(cpu_scale, cpu) = capacity;
}
-#ifdef CONFIG_PROC_SYSCTL
static ssize_t cpu_capacity_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -113,7 +112,6 @@ static int register_cpu_capacity_sysctl(void)
return 0;
}
subsys_initcall(register_cpu_capacity_sysctl);
-#endif
#ifdef CONFIG_OF
struct cpu_efficiency {
--
2.10.0
^ permalink raw reply related
* [PATCH 5/7] arm64: remove wrong CONFIG_PROC_SYSCTL ifdef
From: Juri Lelli @ 2017-01-19 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119143757.14537-1-juri.lelli@arm.com>
The sysfs cpu_capacity entry for each CPU has nothing to do with
PROC_FS, nor it's in /proc/sys path.
Remove such ifdef.
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Reported-and-suggested-by: Sudeep Holla <sudeep.holla@arm.com>
Fixes: be8f185d8af4 ('arm64: add sysfs cpu_capacity attribute')
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
arch/arm64/kernel/topology.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 23e9e13bd2aa..62b370388d72 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -40,7 +40,6 @@ static void set_capacity_scale(unsigned int cpu, unsigned long capacity)
per_cpu(cpu_scale, cpu) = capacity;
}
-#ifdef CONFIG_PROC_SYSCTL
static ssize_t cpu_capacity_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -97,7 +96,6 @@ static int register_cpu_capacity_sysctl(void)
return 0;
}
subsys_initcall(register_cpu_capacity_sysctl);
-#endif
static u32 capacity_scale;
static u32 *raw_capacity;
--
2.10.0
^ permalink raw reply related
* [PATCH 6/7] arm, arm64: factorize common cpu capacity default code
From: Juri Lelli @ 2017-01-19 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119143757.14537-1-juri.lelli@arm.com>
arm and arm64 share lot of code relative to parsing CPU capacity
information from DT, using that information for appropriate scaling and
exposing a sysfs interface for chaging such values at runtime.
Factorize such code in a common place (driver/base/arch_topology.c) in
preparation for further additions.
Suggested-by: Will Deacon <will.deacon@arm.com>
Suggested-by: Mark Rutland <mark.rutland@arm.com>
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
arch/arm/Kconfig | 1 +
arch/arm/kernel/topology.c | 213 ++------------------------------------
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/topology.c | 213 +-------------------------------------
drivers/base/Kconfig | 8 ++
drivers/base/Makefile | 1 +
drivers/base/arch_topology.c | 240 +++++++++++++++++++++++++++++++++++++++++++
7 files changed, 260 insertions(+), 417 deletions(-)
create mode 100644 drivers/base/arch_topology.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 186c4c214e0a..f7059b3a1265 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -19,6 +19,7 @@ config ARM
select EDAC_SUPPORT
select EDAC_ATOMIC_SCRUB
select GENERIC_ALLOCATOR
+ select GENERIC_ARCH_TOPOLOGY
select GENERIC_ATOMIC64 if (CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI)
select GENERIC_CLOCKEVENTS_BROADCAST if SMP
select GENERIC_EARLY_IOREMAP
diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index c760a321935b..51e9ed6439f1 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -43,75 +43,10 @@
* to run the rebalance_domains for all idle cores and the cpu_capacity can be
* updated during this sequence.
*/
-static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
-static DEFINE_MUTEX(cpu_scale_mutex);
-unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
-{
- return per_cpu(cpu_scale, cpu);
-}
-
-static void set_capacity_scale(unsigned int cpu, unsigned long capacity)
-{
- per_cpu(cpu_scale, cpu) = capacity;
-}
-
-static ssize_t cpu_capacity_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct cpu *cpu = container_of(dev, struct cpu, dev);
-
- return sprintf(buf, "%lu\n",
- arch_scale_cpu_capacity(NULL, cpu->dev.id));
-}
-
-static ssize_t cpu_capacity_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t count)
-{
- struct cpu *cpu = container_of(dev, struct cpu, dev);
- int this_cpu = cpu->dev.id, i;
- unsigned long new_capacity;
- ssize_t ret;
-
- if (count) {
- ret = kstrtoul(buf, 0, &new_capacity);
- if (ret)
- return ret;
- if (new_capacity > SCHED_CAPACITY_SCALE)
- return -EINVAL;
-
- mutex_lock(&cpu_scale_mutex);
- for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
- set_capacity_scale(i, new_capacity);
- mutex_unlock(&cpu_scale_mutex);
- }
-
- return count;
-}
-
-static DEVICE_ATTR_RW(cpu_capacity);
-
-static int register_cpu_capacity_sysctl(void)
-{
- int i;
- struct device *cpu;
-
- for_each_possible_cpu(i) {
- cpu = get_cpu_device(i);
- if (!cpu) {
- pr_err("%s: too early to get CPU%d device!\n",
- __func__, i);
- continue;
- }
- device_create_file(cpu, &dev_attr_cpu_capacity);
- }
-
- return 0;
-}
-subsys_initcall(register_cpu_capacity_sysctl);
+extern unsigned long
+arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
+extern void set_capacity_scale(unsigned int cpu, unsigned long capacity);
#ifdef CONFIG_OF
struct cpu_efficiency {
@@ -140,145 +75,9 @@ static unsigned long *__cpu_capacity;
static unsigned long middle_capacity = 1;
static bool cap_from_dt = true;
-static u32 *raw_capacity;
-static bool cap_parsing_failed;
-static u32 capacity_scale;
-
-static int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
-{
- int ret = 1;
- u32 cpu_capacity;
-
- if (cap_parsing_failed)
- return !ret;
-
- ret = of_property_read_u32(cpu_node,
- "capacity-dmips-mhz",
- &cpu_capacity);
- if (!ret) {
- if (!raw_capacity) {
- raw_capacity = kcalloc(num_possible_cpus(),
- sizeof(*raw_capacity),
- GFP_KERNEL);
- if (!raw_capacity) {
- pr_err("cpu_capacity: failed to allocate memory for raw capacities\n");
- cap_parsing_failed = true;
- return ret;
- }
- }
- capacity_scale = max(cpu_capacity, capacity_scale);
- raw_capacity[cpu] = cpu_capacity;
- pr_debug("cpu_capacity: %s cpu_capacity=%u (raw)\n",
- cpu_node->full_name, raw_capacity[cpu]);
- } else {
- if (raw_capacity) {
- pr_err("cpu_capacity: missing %s raw capacity\n",
- cpu_node->full_name);
- pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
- }
- cap_parsing_failed = true;
- kfree(raw_capacity);
- }
-
- return !ret;
-}
-
-static void normalize_cpu_capacity(void)
-{
- u64 capacity;
- int cpu;
-
- if (!raw_capacity || cap_parsing_failed)
- return;
-
- pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
- mutex_lock(&cpu_scale_mutex);
- for_each_possible_cpu(cpu) {
- capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
- / capacity_scale;
- set_capacity_scale(cpu, capacity);
- pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
- cpu, arch_scale_cpu_capacity(NULL, cpu));
- }
- mutex_unlock(&cpu_scale_mutex);
-}
-
-#ifdef CONFIG_CPU_FREQ
-static cpumask_var_t cpus_to_visit;
-static bool cap_parsing_done;
-static void parsing_done_workfn(struct work_struct *work);
-static DECLARE_WORK(parsing_done_work, parsing_done_workfn);
-
-static int
-init_cpu_capacity_callback(struct notifier_block *nb,
- unsigned long val,
- void *data)
-{
- struct cpufreq_policy *policy = data;
- int cpu;
-
- if (cap_parsing_failed || cap_parsing_done)
- return 0;
-
- switch (val) {
- case CPUFREQ_NOTIFY:
- pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
- cpumask_pr_args(policy->related_cpus),
- cpumask_pr_args(cpus_to_visit));
- cpumask_andnot(cpus_to_visit,
- cpus_to_visit,
- policy->related_cpus);
- for_each_cpu(cpu, policy->related_cpus) {
- raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
- policy->cpuinfo.max_freq / 1000UL;
- capacity_scale = max(raw_capacity[cpu], capacity_scale);
- }
- if (cpumask_empty(cpus_to_visit)) {
- normalize_cpu_capacity();
- kfree(raw_capacity);
- pr_debug("cpu_capacity: parsing done\n");
- cap_parsing_done = true;
- schedule_work(&parsing_done_work);
- }
- }
- return 0;
-}
-
-static struct notifier_block init_cpu_capacity_notifier = {
- .notifier_call = init_cpu_capacity_callback,
-};
-
-static int __init register_cpufreq_notifier(void)
-{
- if (cap_parsing_failed)
- return -EINVAL;
-
- if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
- pr_err("cpu_capacity: failed to allocate memory for cpus_to_visit\n");
- return -ENOMEM;
- }
- cpumask_copy(cpus_to_visit, cpu_possible_mask);
-
- return cpufreq_register_notifier(&init_cpu_capacity_notifier,
- CPUFREQ_POLICY_NOTIFIER);
-}
-core_initcall(register_cpufreq_notifier);
-
-static void parsing_done_workfn(struct work_struct *work)
-{
- cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
- CPUFREQ_POLICY_NOTIFIER);
-}
-
-#else
-static int __init free_raw_capacity(void)
-{
- kfree(raw_capacity);
-
- return 0;
-}
-core_initcall(free_raw_capacity);
-#endif
+extern bool cap_parsing_failed;
+extern void normalize_cpu_capacity(void);
+extern int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu);
/*
* Iterate all CPUs' descriptor in DT and compute the efficiency
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 111742126897..7534bb41ee09 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -36,6 +36,7 @@ config ARM64
select EDAC_SUPPORT
select FRAME_POINTER
select GENERIC_ALLOCATOR
+ select GENERIC_ARCH_TOPOLOGY
select GENERIC_CLOCKEVENTS
select GENERIC_CLOCKEVENTS_BROADCAST
select GENERIC_CPU_AUTOPROBE
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 62b370388d72..f629f7524d65 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -21,221 +21,14 @@
#include <linux/sched.h>
#include <linux/slab.h>
#include <linux/string.h>
-#include <linux/cpufreq.h>
#include <asm/cpu.h>
#include <asm/cputype.h>
#include <asm/topology.h>
-static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
-static DEFINE_MUTEX(cpu_scale_mutex);
-
-unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
-{
- return per_cpu(cpu_scale, cpu);
-}
-
-static void set_capacity_scale(unsigned int cpu, unsigned long capacity)
-{
- per_cpu(cpu_scale, cpu) = capacity;
-}
-
-static ssize_t cpu_capacity_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct cpu *cpu = container_of(dev, struct cpu, dev);
-
- return sprintf(buf, "%lu\n",
- arch_scale_cpu_capacity(NULL, cpu->dev.id));
-}
-
-static ssize_t cpu_capacity_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf,
- size_t count)
-{
- struct cpu *cpu = container_of(dev, struct cpu, dev);
- int this_cpu = cpu->dev.id, i;
- unsigned long new_capacity;
- ssize_t ret;
-
- if (count) {
- ret = kstrtoul(buf, 0, &new_capacity);
- if (ret)
- return ret;
- if (new_capacity > SCHED_CAPACITY_SCALE)
- return -EINVAL;
-
- mutex_lock(&cpu_scale_mutex);
- for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
- set_capacity_scale(i, new_capacity);
- mutex_unlock(&cpu_scale_mutex);
- }
-
- return count;
-}
-
-static DEVICE_ATTR_RW(cpu_capacity);
-
-static int register_cpu_capacity_sysctl(void)
-{
- int i;
- struct device *cpu;
-
- for_each_possible_cpu(i) {
- cpu = get_cpu_device(i);
- if (!cpu) {
- pr_err("%s: too early to get CPU%d device!\n",
- __func__, i);
- continue;
- }
- device_create_file(cpu, &dev_attr_cpu_capacity);
- }
-
- return 0;
-}
-subsys_initcall(register_cpu_capacity_sysctl);
-
-static u32 capacity_scale;
-static u32 *raw_capacity;
-static bool cap_parsing_failed;
-
-static void __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
-{
- int ret;
- u32 cpu_capacity;
-
- if (cap_parsing_failed)
- return;
-
- ret = of_property_read_u32(cpu_node,
- "capacity-dmips-mhz",
- &cpu_capacity);
- if (!ret) {
- if (!raw_capacity) {
- raw_capacity = kcalloc(num_possible_cpus(),
- sizeof(*raw_capacity),
- GFP_KERNEL);
- if (!raw_capacity) {
- pr_err("cpu_capacity: failed to allocate memory for raw capacities\n");
- cap_parsing_failed = true;
- return;
- }
- }
- capacity_scale = max(cpu_capacity, capacity_scale);
- raw_capacity[cpu] = cpu_capacity;
- pr_debug("cpu_capacity: %s cpu_capacity=%u (raw)\n",
- cpu_node->full_name, raw_capacity[cpu]);
- } else {
- if (raw_capacity) {
- pr_err("cpu_capacity: missing %s raw capacity\n",
- cpu_node->full_name);
- pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
- }
- cap_parsing_failed = true;
- kfree(raw_capacity);
- }
-}
-
-static void normalize_cpu_capacity(void)
-{
- u64 capacity;
- int cpu;
-
- if (!raw_capacity || cap_parsing_failed)
- return;
-
- pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
- mutex_lock(&cpu_scale_mutex);
- for_each_possible_cpu(cpu) {
- pr_debug("cpu_capacity: cpu=%d raw_capacity=%u\n",
- cpu, raw_capacity[cpu]);
- capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
- / capacity_scale;
- set_capacity_scale(cpu, capacity);
- pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
- cpu, arch_scale_cpu_capacity(NULL, cpu));
- }
- mutex_unlock(&cpu_scale_mutex);
-}
-
-#ifdef CONFIG_CPU_FREQ
-static cpumask_var_t cpus_to_visit;
-static bool cap_parsing_done;
-static void parsing_done_workfn(struct work_struct *work);
-static DECLARE_WORK(parsing_done_work, parsing_done_workfn);
-
-static int
-init_cpu_capacity_callback(struct notifier_block *nb,
- unsigned long val,
- void *data)
-{
- struct cpufreq_policy *policy = data;
- int cpu;
-
- if (cap_parsing_failed || cap_parsing_done)
- return 0;
-
- switch (val) {
- case CPUFREQ_NOTIFY:
- pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
- cpumask_pr_args(policy->related_cpus),
- cpumask_pr_args(cpus_to_visit));
- cpumask_andnot(cpus_to_visit,
- cpus_to_visit,
- policy->related_cpus);
- for_each_cpu(cpu, policy->related_cpus) {
- raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
- policy->cpuinfo.max_freq / 1000UL;
- capacity_scale = max(raw_capacity[cpu], capacity_scale);
- }
- if (cpumask_empty(cpus_to_visit)) {
- normalize_cpu_capacity();
- kfree(raw_capacity);
- pr_debug("cpu_capacity: parsing done\n");
- cap_parsing_done = true;
- schedule_work(&parsing_done_work);
- }
- }
- return 0;
-}
-
-static struct notifier_block init_cpu_capacity_notifier = {
- .notifier_call = init_cpu_capacity_callback,
-};
-
-static int __init register_cpufreq_notifier(void)
-{
- if (cap_parsing_failed)
- return -EINVAL;
-
- if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
- pr_err("cpu_capacity: failed to allocate memory for cpus_to_visit\n");
- return -ENOMEM;
- }
- cpumask_copy(cpus_to_visit, cpu_possible_mask);
-
- return cpufreq_register_notifier(&init_cpu_capacity_notifier,
- CPUFREQ_POLICY_NOTIFIER);
-}
-core_initcall(register_cpufreq_notifier);
-
-static void parsing_done_workfn(struct work_struct *work)
-{
- cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
- CPUFREQ_POLICY_NOTIFIER);
-}
-
-#else
-static int __init free_raw_capacity(void)
-{
- kfree(raw_capacity);
-
- return 0;
-}
-core_initcall(free_raw_capacity);
-#endif
+extern bool cap_parsing_failed;
+extern void normalize_cpu_capacity(void);
+extern int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu);
static int __init get_cpu_for_node(struct device_node *node)
{
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index d718ae4b907a..307ea31187dd 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -339,4 +339,12 @@ config CMA_ALIGNMENT
endif
+config GENERIC_ARCH_TOPOLOGY
+ bool
+ help
+ Enable support for architectures common topology code: e.g., parsing
+ CPU capacity information from DT, usage of such information for
+ appropriate scaling, sysfs interface for changing capacity values at
+ runtime.
+
endmenu
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index f2816f6ff76a..397e5c344e6a 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_SOC_BUS) += soc.o
obj-$(CONFIG_PINCTRL) += pinctrl.o
obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
obj-$(CONFIG_GENERIC_MSI_IRQ_DOMAIN) += platform-msi.o
+obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
obj-y += test/
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
new file mode 100644
index 000000000000..3faf89518892
--- /dev/null
+++ b/drivers/base/arch_topology.c
@@ -0,0 +1,240 @@
+/*
+ * driver/base/arch_topology.c - Arch specific cpu topology information
+ *
+ * Written by: Juri Lelli, ARM Ltd.
+ *
+ * Copyright (C) 2016, ARM Ltd.
+ *
+ * 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 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, GOOD TITLE or
+ * NON INFRINGEMENT. See the GNU General Public License for more
+ * details.
+ *
+ */
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/topology.h>
+
+static DEFINE_MUTEX(cpu_scale_mutex);
+static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
+
+unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
+{
+ return per_cpu(cpu_scale, cpu);
+}
+
+void set_capacity_scale(unsigned int cpu, unsigned long capacity)
+{
+ per_cpu(cpu_scale, cpu) = capacity;
+}
+
+static ssize_t cpu_capacity_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct cpu *cpu = container_of(dev, struct cpu, dev);
+
+ return sprintf(buf, "%lu\n",
+ arch_scale_cpu_capacity(NULL, cpu->dev.id));
+}
+
+static ssize_t cpu_capacity_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ struct cpu *cpu = container_of(dev, struct cpu, dev);
+ int this_cpu = cpu->dev.id, i;
+ unsigned long new_capacity;
+ ssize_t ret;
+
+ if (count) {
+ ret = kstrtoul(buf, 0, &new_capacity);
+ if (ret)
+ return ret;
+ if (new_capacity > SCHED_CAPACITY_SCALE)
+ return -EINVAL;
+
+ mutex_lock(&cpu_scale_mutex);
+ for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
+ set_capacity_scale(i, new_capacity);
+ mutex_unlock(&cpu_scale_mutex);
+ }
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(cpu_capacity);
+
+static int register_cpu_capacity_sysctl(void)
+{
+ int i;
+ struct device *cpu;
+
+ for_each_possible_cpu(i) {
+ cpu = get_cpu_device(i);
+ if (!cpu) {
+ pr_err("%s: too early to get CPU%d device!\n",
+ __func__, i);
+ continue;
+ }
+ device_create_file(cpu, &dev_attr_cpu_capacity);
+ }
+
+ return 0;
+}
+subsys_initcall(register_cpu_capacity_sysctl);
+
+u32 capacity_scale;
+u32 *raw_capacity;
+bool cap_parsing_failed;
+
+void normalize_cpu_capacity(void)
+{
+ u64 capacity;
+ int cpu;
+
+ if (!raw_capacity || cap_parsing_failed)
+ return;
+
+ pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
+ mutex_lock(&cpu_scale_mutex);
+ for_each_possible_cpu(cpu) {
+ pr_debug("cpu_capacity: cpu=%d raw_capacity=%u\n",
+ cpu, raw_capacity[cpu]);
+ capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
+ / capacity_scale;
+ set_capacity_scale(cpu, capacity);
+ pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
+ cpu, arch_scale_cpu_capacity(NULL, cpu));
+ }
+ mutex_unlock(&cpu_scale_mutex);
+}
+
+int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
+{
+ int ret = 1;
+ u32 cpu_capacity;
+
+ if (cap_parsing_failed)
+ return !ret;
+
+ ret = of_property_read_u32(cpu_node,
+ "capacity-dmips-mhz",
+ &cpu_capacity);
+ if (!ret) {
+ if (!raw_capacity) {
+ raw_capacity = kcalloc(num_possible_cpus(),
+ sizeof(*raw_capacity),
+ GFP_KERNEL);
+ if (!raw_capacity) {
+ pr_err("cpu_capacity: failed to allocate memory for raw capacities\n");
+ cap_parsing_failed = true;
+ return ret;
+ }
+ }
+ capacity_scale = max(cpu_capacity, capacity_scale);
+ raw_capacity[cpu] = cpu_capacity;
+ pr_debug("cpu_capacity: %s cpu_capacity=%u (raw)\n",
+ cpu_node->full_name, raw_capacity[cpu]);
+ } else {
+ if (raw_capacity) {
+ pr_err("cpu_capacity: missing %s raw capacity\n",
+ cpu_node->full_name);
+ pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
+ }
+ cap_parsing_failed = true;
+ kfree(raw_capacity);
+ }
+
+ return !ret;
+}
+
+#ifdef CONFIG_CPU_FREQ
+static cpumask_var_t cpus_to_visit;
+static bool cap_parsing_done;
+static void parsing_done_workfn(struct work_struct *work);
+static DECLARE_WORK(parsing_done_work, parsing_done_workfn);
+
+static int
+init_cpu_capacity_callback(struct notifier_block *nb,
+ unsigned long val,
+ void *data)
+{
+ struct cpufreq_policy *policy = data;
+ int cpu;
+
+ if (cap_parsing_failed || cap_parsing_done)
+ return 0;
+
+ switch (val) {
+ case CPUFREQ_NOTIFY:
+ pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
+ cpumask_pr_args(policy->related_cpus),
+ cpumask_pr_args(cpus_to_visit));
+ cpumask_andnot(cpus_to_visit,
+ cpus_to_visit,
+ policy->related_cpus);
+ for_each_cpu(cpu, policy->related_cpus) {
+ raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
+ policy->cpuinfo.max_freq / 1000UL;
+ capacity_scale = max(raw_capacity[cpu], capacity_scale);
+ }
+ if (cpumask_empty(cpus_to_visit)) {
+ normalize_cpu_capacity();
+ kfree(raw_capacity);
+ pr_debug("cpu_capacity: parsing done\n");
+ cap_parsing_done = true;
+ schedule_work(&parsing_done_work);
+ }
+ }
+ return 0;
+}
+
+static struct notifier_block init_cpu_capacity_notifier = {
+ .notifier_call = init_cpu_capacity_callback,
+};
+
+static int __init register_cpufreq_notifier(void)
+{
+ if (cap_parsing_failed)
+ return -EINVAL;
+
+ if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
+ pr_err("cpu_capacity: failed to allocate memory for cpus_to_visit\n");
+ return -ENOMEM;
+ }
+ cpumask_copy(cpus_to_visit, cpu_possible_mask);
+
+ return cpufreq_register_notifier(&init_cpu_capacity_notifier,
+ CPUFREQ_POLICY_NOTIFIER);
+}
+core_initcall(register_cpufreq_notifier);
+
+static void parsing_done_workfn(struct work_struct *work)
+{
+ cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
+ CPUFREQ_POLICY_NOTIFIER);
+}
+
+#else
+static int __init free_raw_capacity(void)
+{
+ kfree(raw_capacity);
+
+ return 0;
+}
+core_initcall(free_raw_capacity);
+#endif
--
2.10.0
^ permalink raw reply related
* [PATCH 7/7] arm,arm64,drivers: reduce scope of cap_parsing_failed
From: Juri Lelli @ 2017-01-19 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119143757.14537-1-juri.lelli@arm.com>
Reduce the scope of cap_parsing_failed (making it static in
drivers/base/arch_topology.c) by slightly changing {arm,arm64} DT
parsing code.
Suggested-by: Morten Rasmussen <morten.rasmussen@arm.com>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---
arch/arm/kernel/topology.c | 3 +--
arch/arm64/kernel/topology.c | 5 +----
drivers/base/arch_topology.c | 4 ++--
3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 51e9ed6439f1..5d4679a5418b 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -75,7 +75,6 @@ static unsigned long *__cpu_capacity;
static unsigned long middle_capacity = 1;
static bool cap_from_dt = true;
-extern bool cap_parsing_failed;
extern void normalize_cpu_capacity(void);
extern int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu);
@@ -164,7 +163,7 @@ static void __init parse_dt_topology(void)
middle_capacity = ((max_capacity / 3)
>> (SCHED_CAPACITY_SHIFT-1)) + 1;
- if (cap_from_dt && !cap_parsing_failed)
+ if (cap_from_dt)
normalize_cpu_capacity();
}
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index f629f7524d65..deb5ebc1bdfe 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -26,7 +26,6 @@
#include <asm/cputype.h>
#include <asm/topology.h>
-extern bool cap_parsing_failed;
extern void normalize_cpu_capacity(void);
extern int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu);
@@ -186,10 +185,8 @@ static int __init parse_dt_topology(void)
* cluster with restricted subnodes.
*/
map = of_get_child_by_name(cn, "cpu-map");
- if (!map) {
- cap_parsing_failed = true;
+ if (!map)
goto out;
- }
ret = parse_cluster(map, 0);
if (ret != 0)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 3faf89518892..cfe51f7e1a3e 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -99,7 +99,7 @@ subsys_initcall(register_cpu_capacity_sysctl);
u32 capacity_scale;
u32 *raw_capacity;
-bool cap_parsing_failed;
+static bool cap_parsing_failed;
void normalize_cpu_capacity(void)
{
@@ -209,7 +209,7 @@ static struct notifier_block init_cpu_capacity_notifier = {
static int __init register_cpufreq_notifier(void)
{
- if (cap_parsing_failed)
+ if (!raw_capacity)
return -EINVAL;
if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
--
2.10.0
^ permalink raw reply related
* [Patch v3 1/2] arm: kernel: Add SMC structure parameter
From: Russell King - ARM Linux @ 2017-01-19 14:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484173918-25090-2-git-send-email-andy.gross@linaro.org>
On Wed, Jan 11, 2017 at 04:31:57PM -0600, Andy Gross wrote:
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index b5abfda..3e28d08 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -72,19 +72,33 @@ struct arm_smccc_res {
> };
>
> /**
> - * arm_smccc_smc() - make SMC calls
> + * struct arm_smccc_quirk - Contains quirk information
> + * id contains quirk identification
> + * state contains the quirk specific information
Given that this is a kerneldoc comment, it should really conform to the
kerneldoc requirements - see Documentation/kernel-doc-nano-HOWTO.txt:
/**
* struct arm_smccc_quirk - Contains quirk information
* @id: quirk identification
* @state: the quirk specific information
> + */
> +struct arm_smccc_quirk {
> + int id;
> + union {
> + unsigned long a6;
> + } state;
> +};
> +
> +/**
> + * __arm_smccc_smc() - make SMC calls
> * @a0-a7: arguments passed in registers 0 to 7
> * @res: result values from registers 0 to 3
> + * @quirk: optional quirk structure
> *
> * This function is used to make SMC calls following SMC Calling Convention.
> * The content of the supplied param are copied to registers 0 to 7 prior
> * to the SMC instruction. The return values are updated with the content
> - * from register 0 to 3 on return from the SMC instruction.
> + * from register 0 to 3 on return from the SMC instruction. An optional
> + * quirk structure provides vendor specific behavior.
It's quite odd to have the result buried in the middle of arguments
passed to a function, but I guess for the sake of simplicity in the
assembly code that's what we need.
Also:
"@quirk points to an arm_smccc_quirk, or NULL when no quirks are required."
And... should this not be const? Are we expecting anyone to modify
the quirk structure?
Thanks.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH 1/2] dt-bindings: arm,gic: Fix binding example for a virt-capable GIC
From: Mark Rutland @ 2017-01-19 14:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484736811-24002-2-git-send-email-marc.zyngier@arm.com>
On Wed, Jan 18, 2017 at 10:53:30AM +0000, Marc Zyngier wrote:
> The joys of copy/paste: the example of a virtualization capable GIC
> in the DT binding was wrong, and propagated to dozens of platforms.
Could you please mention what's wrong (i.e. GICC is impossibly small in
the example).
>
> Oh well. Let's fix the source of the crap before tackling individual
> offenders.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
> index 5393e2a..a3d51ed 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
> @@ -107,11 +107,11 @@ Required properties:
> Example:
>
> interrupt-controller at 2c001000 {
> - compatible = "arm,cortex-a15-gic";
> + compatible = "arm,gic-400";
I'm happy with this change in the spirit of making this more generally
applicable, even if it's not a bug as such. Please mention this as a
related cleanup in the commit message.
With those fixed up:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> #interrupt-cells = <3>;
> interrupt-controller;
> reg = <0x2c001000 0x1000>,
> - <0x2c002000 0x1000>,
> + <0x2c002000 0x2000>,
> <0x2c004000 0x2000>,
> <0x2c006000 0x2000>;
> interrupts = <1 9 0xf04>;
> --
> 2.1.4
>
^ permalink raw reply
* [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration
From: Lorenzo Pieralisi @ 2017-01-19 14:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170106162400.GA8109@red-moon>
Hi Sricharan,
On Fri, Jan 06, 2017 at 04:24:00PM +0000, Lorenzo Pieralisi wrote:
> On Thu, Jan 05, 2017 at 08:21:53PM +0530, Sricharan wrote:
> > Hi,
> >
> > [...]
> >
> > >>>
> > >>> With the thinking of taking this series through, would it be fine if i
> > >>> cleanup the pci configure hanging outside and push it in to
> > >>> of/acpi_iommu configure respectively ? This time with all neeeded for
> > >>> ACPI added as well. Also on the last post of V4, Lorenzo commented
> > >>> that it worked for him, although still the of_match_node equivalent in
> > >>> ACPI has to be added. If i can get that, then i will add that as well
> > >>> to make this complete.
> > >>
> > >> Question: I had a look into this and instead of fiddling about with the
> > >> linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this
> > >> patchset would help remove entirely), I think that the only check we
> > >> need in IORT is, depending on what type of SMMU a given device is
> > >> connected to, to check if the respective SMMU driver is compiled in the
> > >> kernel and it will be probed, _eventually_.
> > >>
> > >> As Robin said, by the time a device is probed the respective SMMU
> > >> devices are already created and registered with IORT kernel code or
> > >> they will never be, so to understand if we should defer probing
> > >> SMMU device creation is _not_ really a problem in ACPI.
> > >>
> > >> To check if a SMMU driver is enabled, do we really need a linker
> > >> table ?
> > >>
> > >> Would not a check based on eg:
> > >>
> > >> /**
> > >> * @type: IOMMU IORT node type of the IOMMU a device is connected to
> > >> */
> > >> static bool iort_iommu_driver_enabled(u8 type)
> > >> {
> > >> switch (type) {
> > >> case ACPI_IORT_SMMU_V3:
> > >> return IS_ENABLED(CONFIG_ARM_SMMU_V3);
> > >
> > >IS_BUILTIN(...)
> > >
> > >> case ACPI_IORT_SMMU:
> > >> return IS_ENABLED(CONFIG_ARM_SMMU);
> > >> default:
> > >> pr_warn("Unknown IORT SMMU type\n");
> > >
> > >Might displaying the actual value be helfpul for debugging a broken IORT
> > >table?
> > >
> > >> return false;
> > >> }
> > >> }
> > >>
> > >> be sufficient (it is a bit gross, agreed, but it is to understand if
> > >> that's all we need) ? Is there anything I am missing ?
> > >>
> > >> Let me know, I will put together a patch for you I really do not
> > >> want to block your series for this trivial niggle.
> > >
> > >Other than that, though, I like it :) IORT has a small, strictly
> > >bounded, set of supported devices, so I really don't see the need to go
> > >overboard putting it on parity with DT when something this neat and
> > >simple will suffice.
> > >
> >
> > Ok sure, looks correct for me as well in whole of the context here.
>
> Ok, I put together a branch where you can find your original series
> plus some ACPI patches for you to test and use:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git iommu/probe-deferral
>
> Feel free to post the additional patches I added along with your series
> (that from what I gather you have reworked already) and please both have a
> look if the deferral mechanism I put in place in ACPI makes sense to you.
Did you have time to make progress on this ? I think it is time we
posted the complete series to aim for 4.11 please, if you need help just
let us know.
Thanks !
Lorenzo
^ permalink raw reply
* [PATCH 1/2] ARM: imx6: fix min/max voltage of anatop 2p5 regulator
From: Philipp Zabel @ 2017-01-19 14:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119142134.8572-1-l.stach@pengutronix.de>
On Thu, 2017-01-19 at 15:21 +0100, Lucas Stach wrote:
> The regulation bound of this regulator are 2.1V to 2.875V, the
> wrong DT values cause the driver to miscalculate the effective
> voltage.
As easily checked with
$ grep vdd2p5 /sys/kernel/debug/regulator/regulator_summary
- vdd2p5 0 0 0 2400mV 0mA 2000mV 2750mV
+ vdd2p5 0 0 0 2500mV 0mA 2000mV 2750mV
> This isn't really an issue right now, as nobody actively changes
> the regulator voltage, but better fix it now.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> arch/arm/boot/dts/imx6qdl.dtsi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 53e6e63cbb02..9313b9af2da8 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -661,8 +661,8 @@
> anatop-vol-bit-shift = <8>;
> anatop-vol-bit-width = <5>;
> anatop-min-bit-val = <0>;
> - anatop-min-voltage = <2000000>;
> - anatop-max-voltage = <2750000>;
> + anatop-min-voltage = <2100000>;
> + anatop-max-voltage = <2875000>;
> };
>
> reg_arm: regulator-vddcore {
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
^ permalink raw reply
* [PATCH 2/2] ARM: imx6: fix regulator constraints on anatop 1p1 and 2p5
From: Philipp Zabel @ 2017-01-19 14:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119142134.8572-2-l.stach@pengutronix.de>
On Thu, 2017-01-19 at 15:21 +0100, Lucas Stach wrote:
> Fix the min/max voltage constraints for the anatop 1p1 and 2p5
> regulator to match the typical operating range mentioned in the
> datasheet.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> arch/arm/boot/dts/imx6qdl.dtsi | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 9313b9af2da8..e9ba244cda54 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -626,8 +626,8 @@
> regulator-1p1 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd1p1";
> - regulator-min-microvolt = <800000>;
> - regulator-max-microvolt = <1375000>;
> + regulator-min-microvolt = <1000000>;
> + regulator-max-microvolt = <1200000>;
> regulator-always-on;
> anatop-reg-offset = <0x110>;
> anatop-vol-bit-shift = <8>;
> @@ -654,7 +654,7 @@
> regulator-2p5 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd2p5";
> - regulator-min-microvolt = <2000000>;
> + regulator-min-microvolt = <2250000>;
> regulator-max-microvolt = <2750000>;
> regulator-always-on;
> anatop-reg-offset = <0x130>;
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
^ permalink raw reply
* [PATCH] arm64/cpufeatures: Enforce inline/const properties of cpus_have_const_cap
From: Marc Zyngier @ 2017-01-19 14:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119143703.GB31594@arm.com>
On 19/01/17 14:37, Will Deacon wrote:
> On Wed, Jan 18, 2017 at 11:58:45AM +0000, Marc Zyngier wrote:
>> Despite being flagged "inline", cpus_have_const_cap may end-up being
>> placed out of line if the compiler decides so. This would be unfortunate,
>> as we want to be able to use this function in HYP, where we need to
>> be 100% sure of what is mapped there. __always_inline seems to be a
>> better choice given the constraint.
>>
>> Also, be a lot tougher on non-const or out-of-range capability values
>> (a non-const cap value shouldn't be used here, and the semantic of an
>> OOR value is at best ill defined). In those two case, BUILD_BUG_ON is
>> what you get.
>>
>> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>> arch/arm64/include/asm/cpufeature.h | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
>> index b4989df..4710469 100644
>> --- a/arch/arm64/include/asm/cpufeature.h
>> +++ b/arch/arm64/include/asm/cpufeature.h
>> @@ -105,10 +105,11 @@ static inline bool cpu_have_feature(unsigned int num)
>> }
>>
>> /* System capability check for constant caps */
>> -static inline bool cpus_have_const_cap(int num)
>> +static __always_inline bool cpus_have_const_cap(int num)
>> {
>> - if (num >= ARM64_NCAPS)
>> - return false;
>> + BUILD_BUG_ON(!__builtin_constant_p(num));
>> + BUILD_BUG_ON(num >= ARM64_NCAPS);
>
> This gives different behaviour to cpus_have_const_cap when compared to
> cpus_have_cap, which I really don't like. What is the current behaviour
> if you pass a non-constant num parameter? Does the kernel actually build?
If your toolchain doesn't support jump labels (gcc 4.8 for example), it
will build. But my point here is that if you're using the _const
version, it should to be an actual constant, within the range of
existing capabilities. Otherwise, I don't really understand what the
semantic of _const means here.
> Maybe it's best to spin a separate patch that makes cpus_have_cap and
> cpus_have_const_cap both use __always_inline, then we can debate the merit
> of the BUILD_BUG_ONs separately.
Sure, will do.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes
From: Uwe Kleine-König @ 2017-01-19 14:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5AdgM9eFnerLRdYxwE7gsOE5OvkWs6rCR4zta1XmXHj1A@mail.gmail.com>
On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
> Hi Uwe,
>
> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-K?nig
> <u.kleine-koenig@pengutronix.de> wrote:
>
> > Would it be nice to add a comment about why this was added? Something to
> > prevent a cleanup like "remove empty nodes and invalid memory
> > configurations".
>
> Do you mean something like this?
>
> /* "chosen" and "memory" nodes are mandatory */
> chosen {};
> memory { device_type = "memory"; reg = <0 0>; };
Not very helpful comment. Something like:
/*
* The decompressor relies on a pre-existing chosen node to be
* available to insert the command line and merge other ATAGS
* info.
*/
Is it difficult to fix the decompressor?
I didn't understood the breakage regarding the memory node good enough
to suggest a comment for that.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH] arm64/cpufeatures: Enforce inline/const properties of cpus_have_const_cap
From: Will Deacon @ 2017-01-19 14:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8dcd71b7-3896-26aa-115c-4d0af8abe916@arm.com>
On Thu, Jan 19, 2017 at 02:42:50PM +0000, Marc Zyngier wrote:
> On 19/01/17 14:37, Will Deacon wrote:
> > On Wed, Jan 18, 2017 at 11:58:45AM +0000, Marc Zyngier wrote:
> >> Despite being flagged "inline", cpus_have_const_cap may end-up being
> >> placed out of line if the compiler decides so. This would be unfortunate,
> >> as we want to be able to use this function in HYP, where we need to
> >> be 100% sure of what is mapped there. __always_inline seems to be a
> >> better choice given the constraint.
> >>
> >> Also, be a lot tougher on non-const or out-of-range capability values
> >> (a non-const cap value shouldn't be used here, and the semantic of an
> >> OOR value is at best ill defined). In those two case, BUILD_BUG_ON is
> >> what you get.
> >>
> >> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >> ---
> >> arch/arm64/include/asm/cpufeature.h | 7 ++++---
> >> 1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> >> index b4989df..4710469 100644
> >> --- a/arch/arm64/include/asm/cpufeature.h
> >> +++ b/arch/arm64/include/asm/cpufeature.h
> >> @@ -105,10 +105,11 @@ static inline bool cpu_have_feature(unsigned int num)
> >> }
> >>
> >> /* System capability check for constant caps */
> >> -static inline bool cpus_have_const_cap(int num)
> >> +static __always_inline bool cpus_have_const_cap(int num)
> >> {
> >> - if (num >= ARM64_NCAPS)
> >> - return false;
> >> + BUILD_BUG_ON(!__builtin_constant_p(num));
> >> + BUILD_BUG_ON(num >= ARM64_NCAPS);
> >
> > This gives different behaviour to cpus_have_const_cap when compared to
> > cpus_have_cap, which I really don't like. What is the current behaviour
> > if you pass a non-constant num parameter? Does the kernel actually build?
>
> If your toolchain doesn't support jump labels (gcc 4.8 for example), it
> will build. But my point here is that if you're using the _const
> version, it should to be an actual constant, within the range of
> existing capabilities. Otherwise, I don't really understand what the
> semantic of _const means here.
There are two things here:
1. GCC can make non-const values constant using a runtime conditional
2. If we treat out-of-range caps as a BUILD_BUG_ON, then we've got
different behaviour with cpus_have_cap, which will return false.
So I don't think that the BUILD_BUG_ON(num >= ARM64_NCAPS) makes an awful
lot of sense, whilst the other BUILD_BUG_ON seems more like a sanity check
on jump labels. That might be justifiable if the build failure is more
obvious than what we currently get, so it's mainly the range check that
I object to.
Will
^ permalink raw reply
* [PATCH v1 0/3] [for arm-soc] mach-tango DT updates for v4.11
From: Marc Gonzalez @ 2017-01-19 14:50 UTC (permalink / raw)
To: linux-arm-kernel
Hello arm-soc maintainers,
Here are my DT updates for tango4.
I also have local changes for NAND and DMA nodes, but the DMA driver
is not upstream yet, so it seems there's no point in sending these.
Marc Gonzalez (3):
ARM: dts: tango4: Add alias for eth0
ARM: dts: tango4: Import MMC nodes
ARM: dts: tango4: Import USB nodes
arch/arm/boot/dts/tango4-common.dtsi | 46 +++++++++++++++++++++++++++++++
arch/arm/boot/dts/tango4-vantage-1172.dts | 5 ++++
2 files changed, 51 insertions(+)
^ permalink raw reply
* [PATCH v1 1/3] ARM: dts: tango4: Add alias for eth0
From: Marc Gonzalez @ 2017-01-19 14:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bd989965-fa8a-a175-5e14-8467f01e6d5b@sigmadesigns.com>
http://elinux.org/Device_Tree_Usage#aliases_Node
Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
arch/arm/boot/dts/tango4-vantage-1172.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/tango4-vantage-1172.dts b/arch/arm/boot/dts/tango4-vantage-1172.dts
index 4cab64cb581e..e27b4a7801ab 100644
--- a/arch/arm/boot/dts/tango4-vantage-1172.dts
+++ b/arch/arm/boot/dts/tango4-vantage-1172.dts
@@ -8,6 +8,7 @@
aliases {
serial = &uart;
+ eth0 = ð0;
};
memory at 80000000 {
--
2.10.0
^ permalink raw reply related
* [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Andrew Lunn @ 2017-01-19 14:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119142820.GA494@kroah.com>
> > > struct dsa_platform_data {
> > > /*
> > > * Reference to a Linux network interface that connects
> > > * to the root switch chip of the tree.
> > > */
> > > struct device *netdev;
>
> This I think is the oddest thing, why do you need to have the "root
> switch" here? You seem to have dropped the next value in this
> structure:
> struct net_device *of_netdev;
We are implementing platform_data for devices which don't support
device tree. When using OF, we don't have any of these issues. We can
go straight to the device.
It is a bit convoluted, but look at
arch/arm/mach-orion5x/rd88f5181l-ge-setup.c. It defines the start of
the dsa_platform_data in that file. It then gets passed through
common.c: orion5x_eth_switch_init() to
arch/arm/plat-orion/common.c:orion_ge00_switch_init() :
void __init orion_ge00_switch_init(struct dsa_platform_data *d)
{
int i;
d->netdev = &orion_ge00.dev;
for (i = 0; i < d->nr_chips; i++)
d->chip[i].host_dev = &orion_ge_mvmdio.dev;
platform_device_register_data(NULL, "dsa", 0, d, sizeof(d));
}
Where we have
static struct platform_device orion_ge00 = {
.name = MV643XX_ETH_NAME,
.id = 0,
.num_resources = 1,
.resource = orion_ge00_resources,
.dev = {
.coherent_dma_mask = DMA_BIT_MASK(32),
},
};
So this is the platform device for the Ethernet device. We cannot go
to the net_device, because it does not exist until this Ethernet
platform device is instantiated.
> Shouldn't you have a bus for RGMII devices? Is that the real problem
> here, you don't have a representation for your RGMII "bus" with a
> controller to bundle everything under (like a USB host controller, it
> bridges from one bus to another).
RGMII is not a bus. It is a point to point link. Normally, it is
between the Ethernet MAC and the Ethernet PHY. But you can also have
it between an Ethernet MAC and another Ethernet MAC. I'm not sure
describing this is a bus would be practical. It would mean every
ethernet driver also becomes a bus driver! Every Ethernet PHY would
become a bus device. That is a huge change, for a few legacy boards
which are not getting converted to device tree.
> If so, why is eth1 not below f1072004.mdio-mi in the heirachy already?
See the initial diagram above. The switch has two parents. It hangs of
an MDIO bus, and you would like to make RGMII also a bus. Can the
device model handle that? I thought it was a tree, not a graph?
Andrew
^ permalink raw reply
* [PATCH 6/7] arm, arm64: factorize common cpu capacity default code
From: Russell King - ARM Linux @ 2017-01-19 14:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170119143757.14537-7-juri.lelli@arm.com>
On Thu, Jan 19, 2017 at 02:37:56PM +0000, Juri Lelli wrote:
> +extern unsigned long
> +arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> +extern void set_capacity_scale(unsigned int cpu, unsigned long capacity);
These should be in a header file (please run your code through sparse).
> +extern bool cap_parsing_failed;
> +extern void normalize_cpu_capacity(void);
> +extern int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu);
Same for these.
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> new file mode 100644
> index 000000000000..3faf89518892
> --- /dev/null
> +++ b/drivers/base/arch_topology.c
> @@ -0,0 +1,240 @@
> +/*
> + * driver/base/arch_topology.c - Arch specific cpu topology information
> + *
> + * Written by: Juri Lelli, ARM Ltd.
> + *
> + * Copyright (C) 2016, ARM Ltd.
> + *
> + * 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 as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
The files that you've taken this code from are GPLv2, but you've thrown
a GPLv2+ header on a file that's merely a copy of the original code.
As some of the code you've moved to this new file is from Nicolas and
Vincent, you need to seek their approval to make this change of license
terms, or keep the original license terms.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH 2/2] [media] exynos-gsc: Fix imprecise external abort due disabled power domain
From: Javier Martinez Canillas @ 2017-01-19 14:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cc9c6837-7141-b63c-ddf6-68252493df11@samsung.com>
Hello Marek,
Thanks a lot for your feedback.
On 01/19/2017 11:17 AM, Marek Szyprowski wrote:
> Hi Javier,
>
> On 2017-01-18 01:30, Javier Martinez Canillas wrote:
>> Commit 15f90ab57acc ("[media] exynos-gsc: Make driver functional when
>> CONFIG_PM is unset") removed the implicit dependency that the driver
>> had with CONFIG_PM, since it relied on the config option to be enabled.
>>
>> In order to work with !CONFIG_PM, the GSC reset logic that happens in
>> the runtime resume callback had to be executed on the probe function.
>>
>> The problem is that if CONFIG_PM is enabled, the power domain for the
>> GSC could be disabled and so an attempt to write to the GSC_SW_RESET
>> register leads to an unhandled fault / imprecise external abort error:
>
> Driver core ensures that driver's probe() is called with respective power
> domain turned on, so this is not the right reason for the proposed change.
>
Ok, I misunderstood the relationship between runtime PM and the power domains
then. I thought the power domains were only powered on when the runtime PM
framework resumed an associated device (i.e: pm_runtime_get_sync() is called).
But even if this isn't the case, shouldn't the reset in probe only be needed
if CONFIG_PM isn't enabled? (IOW, $SUBJECT but with another commit message).
>> [ 10.178825] Unhandled fault: imprecise external abort (0x1406) at 0x00000000
>> [ 10.186982] pgd = ed728000
>> [ 10.190847] [00000000] *pgd=00000000
>> [ 10.195553] Internal error: : 1406 [#1] PREEMPT SMP ARM
>> [ 10.229761] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>> [ 10.237134] task: ed49e400 task.stack: ed724000
>> [ 10.242934] PC is at gsc_wait_reset+0x5c/0x6c [exynos_gsc]
>> [ 10.249710] LR is at gsc_probe+0x300/0x33c [exynos_gsc]
>> [ 10.256139] pc : [<bf2429e0>] lr : [<bf240734>] psr: 60070013
>> [ 10.256139] sp : ed725d30 ip : 00000000 fp : 00000001
>> [ 10.271492] r10: eea74800 r9 : ecd6a2c0 r8 : ed7d8854
>> [ 10.277912] r7 : ed7d8c08 r6 : ed7d8810 r5 : ffff8ecd r4 : c0c03900
>> [ 10.285664] r3 : 00000000 r2 : 00000001 r1 : ed7d8b98 r0 : ed7d8810
>>
>> So only do a GSC reset if CONFIG_PM is disabled, since if is enabled the
>> runtime PM resume callback will be called by the VIDIOC_STREAMON ioctl,
>> making the reset in probe unneeded.
>>
>> Fixes: 15f90ab57acc ("[media] exynos-gsc: Make driver functional when CONFIG_PM is unset")
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>
> Frankly, I don't get why this change is needed.
>
Yes, it seems $SUBJECT is just papering over the real issue. There's
something really wrong with the Exynos power domains, I see that PDs
can't be disabled by the genpd framework, exynos_pd_power_off() fail:
# dmesg | grep power-domain
[ 4.893318] Power domain power-domain at 10044020 disable failed
[ 4.893342] Power domain power-domain at 10044120 disable failed
[ 4.893711] Power domain power-domain at 10044000 disable failed
[ 12.690052] Power domain power-domain at 10044000 disable failed
[ 12.703963] Power domain power-domain at 10044000 disable failed
So PD are kept on even when unused / attached devices are suspended.
Only the mfc_pd (power-domain at 10044060) is correctly turned off.
# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
domain status slaves
/device runtime status
----------------------------------------------------------------------
power-domain at 100440C0 on
/devices/platform/soc/14450000.mixer active
/devices/platform/soc/14530000.hdmi active
power-domain at 10044120 on
power-domain at 10044060 off-0
/devices/platform/soc/11000000.codec suspended
power-domain at 10044020 on
power-domain at 10044000 on
/devices/platform/soc/13e00000.video-scaler suspended
/devices/platform/soc/13e10000.video-scaler suspended
Also when removing the exynos_gsc driver, I get the same error:
# rmmod s5p_mfc
[ 106.405972] s5p-mfc 11000000.codec: Removing 11000000.codec
# rmmod exynos_gsc
[ 227.008559] Unhandled fault: imprecise external abort (0x1c06) at 0x00048e14
[ 227.015116] pgd = ed5dc000
[ 227.017213] [00048e14] *pgd=b17c6835
[ 227.020889] Internal error: : 1c06 [#1] PREEMPT SMP ARM
...
[ 227.241585] [<bf2429bc>] (gsc_wait_reset [exynos_gsc]) from [<bf24009c>] (gsc_runtime_resume+0x9c/0xec [exynos_gsc])
[ 227.252331] [<bf24009c>] (gsc_runtime_resume [exynos_gsc]) from [<c042e488>] (genpd_runtime_resume+0x120/0x1d4)
[ 227.262294] [<c042e488>] (genpd_runtime_resume) from [<c04241c0>] (__rpm_callback+0xc8/0x218)
# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
domain status slaves
/device runtime status
----------------------------------------------------------------------
power-domain at 100440C0 on
/devices/platform/soc/14450000.mixer active
/devices/platform/soc/14530000.hdmi active
power-domain at 10044120 on
power-domain at 10044060 off-0
power-domain at 10044020 on
power-domain at 10044000 on
/devices/platform/soc/13e00000.video-scaler suspended
/devices/platform/soc/13e10000.video-scaler resuming
This seems to be caused by some needed clocks to access the power domains
to be gated, since I don't get these erros when passing clk_ignore_unused
as parameter in the kernel command line.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* [PATCH v14 3/5] tee: add OP-TEE driver
From: Jens Wiklander @ 2017-01-19 14:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <12500736.cNG1jjvVpX@wuerfel>
On Wed, Jan 18, 2017 at 05:28:17PM +0100, Arnd Bergmann wrote:
> On Wednesday, January 18, 2017 1:58:14 PM CET Jens Wiklander wrote:
> > Adds a OP-TEE driver which also can be compiled as a loadable module.
> >
> > * Targets ARM and ARM64
> > * Supports using reserved memory from OP-TEE as shared memory
> > * Probes OP-TEE version using SMCs
> > * Accepts requests on privileged and unprivileged device
> > * Uses OPTEE message protocol version 2 to communicate with secure world
>
> I had not really followed the last versions, and I've looked through
> it now for things that seemed odd to me, either because I don't understand
> them or because they could be improved. I'll try to read it again after
> I've seen clarifications on these points.
>
> Generally speaking I haven't seen any show-stoppers so far.
>
> > +struct optee_call_waiter {
> > + struct list_head list_node;
> > + struct completion c;
> > + bool completed;
> > +};
>
> It seems wrong to have both a 'struct completion' and 'bool completed' here,
> as completion already contains such a flag and is designed to update that
> atomically.
You're right, I'll remove the 'bool completed'.
>
> > +static void optee_cq_complete_one(struct optee_call_queue *cq)
> > +{
> > + struct optee_call_waiter *w;
> > +
> > + list_for_each_entry(w, &cq->waiters, list_node) {
> > + if (!w->completed) {
> > + complete(&w->c);
> > + w->completed = true;
> > + break;
> > + }
> > + }
> > +}
> > +
> > +static void optee_cq_wait_final(struct optee_call_queue *cq,
> > + struct optee_call_waiter *w)
> > +{
> > + mutex_lock(&cq->mutex);
> > +
> > + /* Get out of the list */
> > + list_del(&w->list_node);
> > +
> > + optee_cq_complete_one(cq);
> > + /*
> > + * If we're completed we've got a completion that some other task
> > + * could have used instead.
> > + */
> > + if (w->completed)
> > + optee_cq_complete_one(cq);
> > +
> > + mutex_unlock(&cq->mutex);
> > +}
>
> This deserves some more comments: the function name suggests that you are
> waiting for a specific optee_call_waiter, but then it calls
> optee_cq_complete_one(), which unconditionally completes the first
> incomplete completion and it never waits.
OK, I'm updating these functions with more comments. The purpose of
these functions is to deal with resource shortage in secure world.
There's a limit on how many threads that execute concurrently in secure
world. optee_cq_wait_for_completion() waits for any task returning from
a call to secure world, optee_cq_complete_one() doesn't care who's
completed as long as tasks aren't stuck when there's available resources
in secure world.
>
> > +static struct tee_shm_pool *
> > +optee_config_shm_ioremap(struct device *dev, optee_invoke_fn *invoke_fn,
> > + void __iomem **ioremaped_shm)
> > +{
> > + union {
> > + struct arm_smccc_res smccc;
> > + struct optee_smc_get_shm_config_result result;
> > + } res;
> > + struct tee_shm_pool *pool;
> > + unsigned long vaddr;
> > + phys_addr_t paddr;
> > + size_t size;
> > + phys_addr_t begin;
> > + phys_addr_t end;
> > + void __iomem *va;
> > + struct tee_shm_pool_mem_info priv_info;
> > + struct tee_shm_pool_mem_info dmabuf_info;
> > +
> > + invoke_fn(OPTEE_SMC_GET_SHM_CONFIG, 0, 0, 0, 0, 0, 0, 0, &res.smccc);
> > + if (res.result.status != OPTEE_SMC_RETURN_OK) {
> > + dev_info(dev, "shm service not available\n");
> > + return ERR_PTR(-ENOENT);
> > + }
> > +
> > + if (res.result.settings != OPTEE_SMC_SHM_CACHED) {
> > + dev_err(dev, "only normal cached shared memory supported\n");
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + begin = roundup(res.result.start, PAGE_SIZE);
> > + end = rounddown(res.result.start + res.result.size, PAGE_SIZE);
> > + paddr = begin;
> > + size = end - begin;
> > +
> > + if (size < 2 * OPTEE_SHM_NUM_PRIV_PAGES * PAGE_SIZE) {
> > + dev_err(dev, "too small shared memory area\n");
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + va = ioremap_cache(paddr, size);
> > + if (!va) {
> > + dev_err(dev, "shared memory ioremap failed\n");
> > + return ERR_PTR(-EINVAL);
> > + }
> > + vaddr = (unsigned long)va;
>
> I think you should call memremap() instead of ioremap_cache() here and assume
> that you are talking to actual RAM.
Thanks, I'll update.
>
> > +static int __init optee_driver_init(void)
> > +{
> > + struct device_node *node;
> > +
> > + /*
> > + * Preferred path is /firmware/optee, but it's the matching that
> > + * matters.
> > + */
> > + for_each_matching_node(node, optee_match)
> > + of_platform_device_create(node, NULL, NULL);
> > +
> > + return platform_driver_register(&optee_driver);
> > +}
> > +module_init(optee_driver_init);
> > +
> > +static void __exit optee_driver_exit(void)
> > +{
> > + platform_driver_unregister(&optee_driver);
> > +}
> > +module_exit(optee_driver_exit);
>
> What is the platform driver good for if the same module has to create the
> platform devices itself?
The platform device(s) are created here because the optee node is below
"/firmware" instead of the root where it would have had the platform
device created automatically.
I think it's useful to be able to unload the module, the early reviews
of this patch set was much focused around that. Regardless I'll need
some device as parent for the devices created during optee_probe() and
using a platform device for that seems natural.
I'd rather keep the platform driver. Perhaps some variant of the pattern
in qcom_scm_init() (drivers/firmware/qcom_scm.c) is useful, except that
I need to find out what to do about the life cycle of the objects
created with of_platform_populate().
>
> I'd just skip it and do
>
> for_each_matching_node(node, optee_match)
> optee_probe(node);
>
> I also suspect that module unloading is broken here if you don't clean
> up the platform devices in the end, so you should already remove the
> exit function to prevent unloading.
Does the platform devices really need cleaning? I mean
of_platform_default_populate_init() creates a bunch of platform devices
which are just left there even if unused. Here we're doing the same
thing except that we're doing it for a specific node in the DT.
>
> > +struct optee_msg_arg {
> > + u32 cmd;
> > + u32 func;
> > + u32 session;
> > + u32 cancel_id;
> > + u32 pad;
> > + u32 ret;
> > + u32 ret_origin;
> > + u32 num_params;
> > +
> > + /*
> > + * this struct is 8 byte aligned since the 'struct optee_msg_param'
> > + * which follows requires 8 byte alignment.
> > + *
> > + * Commented out element used to visualize the layout dynamic part
> > + * of the struct. This field is not available at all if
> > + * num_params == 0.
> > + *
> > + * params is accessed through the macro OPTEE_MSG_GET_PARAMS
> > + *
> > + * struct optee_msg_param params[num_params];
> > + */
> > +} __aligned(8);
> > +
> > +/**
> > + * OPTEE_MSG_GET_PARAMS - return pointer to struct optee_msg_param *
> > + *
> > + * @x: Pointer to a struct optee_msg_arg
> > + *
> > + * Returns a pointer to the params[] inside a struct optee_msg_arg.
> > + */
> > +#define OPTEE_MSG_GET_PARAMS(x) \
> > + (struct optee_msg_param *)(((struct optee_msg_arg *)(x)) + 1)
>
> If you make the last member of optee_msg_arg
>
> struct optee_msg_param params[0];
>
> then you can remove both the macro here and the alignment attribute.
OK.
>
> > +/*****************************************************************************
> > + * Part 2 - requests from normal world
> > + *****************************************************************************/
> > +
> > +/*
> > + * Return the following UID if using API specified in this file without
> > + * further extensions:
> > + * 384fb3e0-e7f8-11e3-af63-0002a5d5c51b.
> > + * Represented in 4 32-bit words in OPTEE_MSG_UID_0, OPTEE_MSG_UID_1,
> > + * OPTEE_MSG_UID_2, OPTEE_MSG_UID_3.
> > + */
> > +#define OPTEE_MSG_UID_0 0x384fb3e0
> > +#define OPTEE_MSG_UID_1 0xe7f811e3
> > +#define OPTEE_MSG_UID_2 0xaf630002
> > +#define OPTEE_MSG_UID_3 0xa5d5c51b
> > +#define OPTEE_MSG_FUNCID_CALLS_UID 0xFF01
> > +
> > +/*
> > + * Returns 2.0 if using API specified in this file without further
> > + * extensions. Represented in 2 32-bit words in OPTEE_MSG_REVISION_MAJOR
> > + * and OPTEE_MSG_REVISION_MINOR
> > + */
> > +#define OPTEE_MSG_REVISION_MAJOR 2
> > +#define OPTEE_MSG_REVISION_MINOR 0
> > +#define OPTEE_MSG_FUNCID_CALLS_REVISION 0xFF03
> > +
> > +/*
> > + * Get UUID of Trusted OS.
> > + *
> > + * Used by non-secure world to figure out which Trusted OS is installed.
> > + * Note that returned UUID is the UUID of the Trusted OS, not of the API.
> > + *
> > + * Returns UUID in 4 32-bit words in the same way as
> > + * OPTEE_MSG_FUNCID_CALLS_UID described above.
> > + */
> > +#define OPTEE_MSG_OS_OPTEE_UUID_0 0x486178e0
> > +#define OPTEE_MSG_OS_OPTEE_UUID_1 0xe7f811e3
> > +#define OPTEE_MSG_OS_OPTEE_UUID_2 0xbc5e0002
> > +#define OPTEE_MSG_OS_OPTEE_UUID_3 0xa5d5c51b
> > +#define OPTEE_MSG_FUNCID_GET_OS_UUID 0x0000
> > +
> > +/*
> > + * Get revision of Trusted OS.
> > + *
> > + * Used by non-secure world to figure out which version of the Trusted OS
> > + * is installed. Note that the returned revision is the revision of the
> > + * Trusted OS, not of the API.
> > + *
> > + * Returns revision in 2 32-bit words in the same way as
> > + * OPTEE_MSG_CALLS_REVISION described above.
> > + */
> > +#define OPTEE_MSG_OS_OPTEE_REVISION_MAJOR 1
> > +#define OPTEE_MSG_OS_OPTEE_REVISION_MINOR 0
> > +#define OPTEE_MSG_FUNCID_GET_OS_REVISION 0x0001
>
> Just for my understanding, what is the significance of these numbers,
> i.e. which code (user space, kernel driver, trusted OS) provides
> the uuid and which one provides the version? The code comments almost
> make sense to me, but I don't see why specific versions are listed
> in this header.
You're right, OPTEE_MSG_OS_OPTEE_REVISION_* should be removed. The
actual version the secure OS is of a mostly informational nature. The
same goes the OS UUID, but I suppose the actual UUID used by the
upstream version of OP-TEE OS could be interesting to know.
>
> What is the expected behavior when one side reports a version that
> is unknown? Can one side claim to be backwards compatible with
> a previous version, or does each new version need support on
> all three sides?
The UUID and version of the message protocol are important to match
correctly as otherwise it could mean that there's something unexpected
in secure world that following the message protocol would be undefined
behaviour. All changes to the message protocol should be backwards
compatible in the sense that the driver and secure world need to
negotiate eventual extensions while probing. That's what we're doing in
optee_msg_exchange_capabilities().
>
> > diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
> > new file mode 100644
> > index 000000000000..0b9c1a2accd0
> > --- /dev/null
> > +++ b/drivers/tee/optee/rpc.c
> > +static void handle_rpc_func_cmd_wq(struct optee *optee,
> > + struct optee_msg_arg *arg)
> > +{
> > + struct optee_msg_param *params;
> > +
> > + if (arg->num_params != 1)
> > + goto bad;
> > +
> > + params = OPTEE_MSG_GET_PARAMS(arg);
> > + if ((params->attr & OPTEE_MSG_ATTR_TYPE_MASK) !=
> > + OPTEE_MSG_ATTR_TYPE_VALUE_INPUT)
> > + goto bad;
> > +
> > + switch (params->u.value.a) {
> > + case OPTEE_MSG_RPC_WAIT_QUEUE_SLEEP:
> > + wq_sleep(&optee->wait_queue, params->u.value.b);
> > + break;
> > + case OPTEE_MSG_RPC_WAIT_QUEUE_WAKEUP:
> > + wq_wakeup(&optee->wait_queue, params->u.value.b);
> > + break;
> > + default:
> > + goto bad;
> > + }
> > +
> > + arg->ret = TEEC_SUCCESS;
> > + return;
> > +bad:
> > + arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > +}
> > +
>
> I'm trying to understand what this is good for. What I can see is that
> you have a user space process calling into the kernel asking the tee
> to do some command, and then the tee can ask the kernel to wait for
> something to happen, or notify it that something has happened.
>
> If we wait here, the user process gets suspended until this has
> actually happened.
>
> Am I reading this correctly? If yes, what is the intended use case?
> Is there some process that is meant to always wait here? What
> if we ever need to wait for more than one thing at a time (think
> select or poll?)
I'm updating the comments for OPTEE_MSG_RPC_CMD_WAIT_QUEUE with:
"If secure world need to wait for a secure world mutex it issues a sleep
request instead of spinning in secure world. Conversely is a wakeup
request issued when a secure world mutex with a thread waiting thread is
unlocked."
The way we're waking up a sleeping thread is a bit limiting in some
circumstances.
One case is where we need to do it from a secure interrupt handler.
Because there's no way of doing this kind of RPC to normal world from a
secure interrupt handler. In that case it wouldn't be mutex the thread
is waiting for though.
Another case is where there's several guest running in the system and
more than one guests has access to secure world. If guest2 waits for
mutex which guest1 is releasing, how can guest2 be notified? Normal RPC
is impossible here also.
The only way around this limitation I've come up with so far is by doing
the wakeup via a software generated interrupt destined to the correct
guest. However this problem is beyond the scope of this patch set.
>
> > + params = OPTEE_MSG_GET_PARAMS(arg);
> > + if ((params->attr & OPTEE_MSG_ATTR_TYPE_MASK) !=
> > + OPTEE_MSG_ATTR_TYPE_VALUE_INPUT)
> > + goto bad;
> > +
> > + msec_to_wait = params->u.value.a;
> > +
> > + /* set task's state to interruptible sleep */
> > + set_current_state(TASK_INTERRUPTIBLE);
> > +
> > + /* take a nap */
> > + schedule_timeout(msecs_to_jiffies(msec_to_wait));
>
> This can be done simpler with msleep();
OK.
Thanks,
Jens
^ 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