All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-04-20 14:43 ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	juri.lelli, broonie, gregkh

Hi,

arm and arm64 topology.c share a lot of code related to parsing of capacity
information. This is v4 of a solution [1] (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 4 patches are actually fixes for the current code.

Patch 5 is the actual refactoring.

Patch 6 removes one of the extern symbols by changing a bit the now common
code.

Patch 7 removes the remaining externs (as required by Russell during v1 review)
by creating a new header file include/linux/arch_topology.h and including that
from arm, arm64 and drivers.

Last patch addresses Dietmar's comments to v1 and adds a 'atd_' prefix to
interfaces exported by drivers code and used by arch (and potentially others in
the future).

Changes from v3:

 - rebase on top of 4.11-rc7
 - addressed Vincent's comment on patch 3/8
 - removed v3 6/9 as discussed with Russell
 - added Russell and Catalin's Acked-by

The set is based on top of linux/master (4.11-rc7 f61143c45077) and it is also
available from:

 git://linux-arm.org/linux-jl.git upstream/default_caps_factorize-v4

Best,

- Juri

[1] v1 - https://marc.info/?l=linux-kernel&m=148483680119355&w=2
    v2 - https://marc.info/?l=linux-kernel&m=148663344018205&w=2
    v3 - http://marc.info/?l=linux-kernel&m=149062080701399&w=2

Juri Lelli (8):
  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
  arm, arm64: factorize common cpu capacity default code
  arm,arm64,drivers: reduce scope of cap_parsing_failed
  arm,arm64,drivers: move externs in a new header file
  arm,arm64,drivers: add a prefix to drivers arch_topology interfaces

 Documentation/ABI/testing/sysfs-devices-system-cpu |   7 +
 Documentation/devicetree/bindings/arm/cpus.txt     |   4 +-
 arch/arm/Kconfig                                   |   1 +
 arch/arm/kernel/topology.c                         | 221 +------------------
 arch/arm64/Kconfig                                 |   1 +
 arch/arm64/kernel/topology.c                       | 226 +------------------
 drivers/base/Kconfig                               |   8 +
 drivers/base/Makefile                              |   1 +
 drivers/base/arch_topology.c                       | 243 +++++++++++++++++++++
 include/linux/arch_topology.h                      |  17 ++
 10 files changed, 290 insertions(+), 439 deletions(-)
 create mode 100644 drivers/base/arch_topology.c
 create mode 100644 include/linux/arch_topology.h

-- 
2.10.0

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-04-20 14:43 ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-lFZ/pmaqli7XmaaqVzeoHQ, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, morten.rasmussen-5wv7dgnIgG8,
	dietmar.eggemann-5wv7dgnIgG8, juri.lelli-5wv7dgnIgG8,
	broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r

Hi,

arm and arm64 topology.c share a lot of code related to parsing of capacity
information. This is v4 of a solution [1] (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 4 patches are actually fixes for the current code.

Patch 5 is the actual refactoring.

Patch 6 removes one of the extern symbols by changing a bit the now common
code.

Patch 7 removes the remaining externs (as required by Russell during v1 review)
by creating a new header file include/linux/arch_topology.h and including that
from arm, arm64 and drivers.

Last patch addresses Dietmar's comments to v1 and adds a 'atd_' prefix to
interfaces exported by drivers code and used by arch (and potentially others in
the future).

Changes from v3:

 - rebase on top of 4.11-rc7
 - addressed Vincent's comment on patch 3/8
 - removed v3 6/9 as discussed with Russell
 - added Russell and Catalin's Acked-by

The set is based on top of linux/master (4.11-rc7 f61143c45077) and it is also
available from:

 git://linux-arm.org/linux-jl.git upstream/default_caps_factorize-v4

Best,

- Juri

[1] v1 - https://marc.info/?l=linux-kernel&m=148483680119355&w=2
    v2 - https://marc.info/?l=linux-kernel&m=148663344018205&w=2
    v3 - http://marc.info/?l=linux-kernel&m=149062080701399&w=2

Juri Lelli (8):
  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
  arm, arm64: factorize common cpu capacity default code
  arm,arm64,drivers: reduce scope of cap_parsing_failed
  arm,arm64,drivers: move externs in a new header file
  arm,arm64,drivers: add a prefix to drivers arch_topology interfaces

 Documentation/ABI/testing/sysfs-devices-system-cpu |   7 +
 Documentation/devicetree/bindings/arm/cpus.txt     |   4 +-
 arch/arm/Kconfig                                   |   1 +
 arch/arm/kernel/topology.c                         | 221 +------------------
 arch/arm64/Kconfig                                 |   1 +
 arch/arm64/kernel/topology.c                       | 226 +------------------
 drivers/base/Kconfig                               |   8 +
 drivers/base/Makefile                              |   1 +
 drivers/base/arch_topology.c                       | 243 +++++++++++++++++++++
 include/linux/arch_topology.h                      |  17 ++
 10 files changed, 290 insertions(+), 439 deletions(-)
 create mode 100644 drivers/base/arch_topology.c
 create mode 100644 include/linux/arch_topology.h

-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-04-20 14:43 ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 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 is v4 of a solution [1] (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 4 patches are actually fixes for the current code.

Patch 5 is the actual refactoring.

Patch 6 removes one of the extern symbols by changing a bit the now common
code.

Patch 7 removes the remaining externs (as required by Russell during v1 review)
by creating a new header file include/linux/arch_topology.h and including that
from arm, arm64 and drivers.

Last patch addresses Dietmar's comments to v1 and adds a 'atd_' prefix to
interfaces exported by drivers code and used by arch (and potentially others in
the future).

Changes from v3:

 - rebase on top of 4.11-rc7
 - addressed Vincent's comment on patch 3/8
 - removed v3 6/9 as discussed with Russell
 - added Russell and Catalin's Acked-by

The set is based on top of linux/master (4.11-rc7 f61143c45077) and it is also
available from:

 git://linux-arm.org/linux-jl.git upstream/default_caps_factorize-v4

Best,

- Juri

[1] v1 - https://marc.info/?l=linux-kernel&m=148483680119355&w=2
    v2 - https://marc.info/?l=linux-kernel&m=148663344018205&w=2
    v3 - http://marc.info/?l=linux-kernel&m=149062080701399&w=2

Juri Lelli (8):
  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
  arm, arm64: factorize common cpu capacity default code
  arm,arm64,drivers: reduce scope of cap_parsing_failed
  arm,arm64,drivers: move externs in a new header file
  arm,arm64,drivers: add a prefix to drivers arch_topology interfaces

 Documentation/ABI/testing/sysfs-devices-system-cpu |   7 +
 Documentation/devicetree/bindings/arm/cpus.txt     |   4 +-
 arch/arm/Kconfig                                   |   1 +
 arch/arm/kernel/topology.c                         | 221 +------------------
 arch/arm64/Kconfig                                 |   1 +
 arch/arm64/kernel/topology.c                       | 226 +------------------
 drivers/base/Kconfig                               |   8 +
 drivers/base/Makefile                              |   1 +
 drivers/base/arch_topology.c                       | 243 +++++++++++++++++++++
 include/linux/arch_topology.h                      |  17 ++
 10 files changed, 290 insertions(+), 439 deletions(-)
 create mode 100644 drivers/base/arch_topology.c
 create mode 100644 include/linux/arch_topology.h

-- 
2.10.0

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 1/8] Documentation: arm: fix wrong reference number in DT definition
  2017-04-20 14:43 ` Juri Lelli
@ 2017-04-20 14:43   ` Juri Lelli
  -1 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	juri.lelli, broonie, gregkh

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>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 698ad1f097fa..83ec3fa0a05e 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -248,7 +248,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.
 
@@ -475,5 +475,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	[flat|nested] 63+ messages in thread
* [PATCH v4 1/8] Documentation: arm: fix wrong reference number in DT definition
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-arm-kernel

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>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 698ad1f097fa..83ec3fa0a05e 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -248,7 +248,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.
 
@@ -475,5 +475,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	[flat|nested] 63+ messages in thread
* [PATCH v4 2/8] Documentation/ABI: add information about cpu_capacity
  2017-04-20 14:43 ` Juri Lelli
  (?)
@ 2017-04-20 14:43   ` Juri Lelli
  -1 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	juri.lelli, broonie, gregkh

/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>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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	[flat|nested] 63+ messages in thread
* [PATCH v4 2/8] Documentation/ABI: add information about cpu_capacity
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: mark.rutland, devicetree, lorenzo.pieralisi, vincent.guittot,
	juri.lelli, linux-pm, peterz, catalin.marinas, broonie,
	will.deacon, gregkh, dietmar.eggemann, robh+dt, sudeep.holla,
	linux, morten.rasmussen, linux-arm-kernel

/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>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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	[flat|nested] 63+ messages in thread
* [PATCH v4 2/8] Documentation/ABI: add information about cpu_capacity
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-arm-kernel

/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>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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	[flat|nested] 63+ messages in thread
* [PATCH v4 3/8] arm: fix return value of parse_cpu_capacity
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	juri.lelli, broonie, gregkh

parse_cpu_capacity() has to return 0 on failure, but it currently returns
1 instead if raw_capacity kcalloc failed.

Fix it (by directly returning 0).

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>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---

Changes from v3:

 - directly return 0 on failure (as pointed out by Vincent)
---
 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 f8a3ab82e77f..1b8ec3054642 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -166,7 +166,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 0;
 			}
 		}
 		capacity_scale = max(cpu_capacity, capacity_scale);
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* [PATCH v4 3/8] arm: fix return value of parse_cpu_capacity
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-lFZ/pmaqli7XmaaqVzeoHQ, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, morten.rasmussen-5wv7dgnIgG8,
	dietmar.eggemann-5wv7dgnIgG8, juri.lelli-5wv7dgnIgG8,
	broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r

parse_cpu_capacity() has to return 0 on failure, but it currently returns
1 instead if raw_capacity kcalloc failed.

Fix it (by directly returning 0).

Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Reported-by: Morten Rasmussen <morten.rasmussen-5wv7dgnIgG8@public.gmane.org>
Fixes: 06073ee26775 ('ARM: 8621/3: parse cpu capacity-dmips-mhz from DT')
Signed-off-by: Juri Lelli <juri.lelli-5wv7dgnIgG8@public.gmane.org>
Acked-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
---

Changes from v3:

 - directly return 0 on failure (as pointed out by Vincent)
---
 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 f8a3ab82e77f..1b8ec3054642 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -166,7 +166,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 0;
 			}
 		}
 		capacity_scale = max(cpu_capacity, capacity_scale);
-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* [PATCH v4 3/8] arm: fix return value of parse_cpu_capacity
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-arm-kernel

parse_cpu_capacity() has to return 0 on failure, but it currently returns
1 instead if raw_capacity kcalloc failed.

Fix it (by directly returning 0).

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>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---

Changes from v3:

 - directly return 0 on failure (as pointed out by Vincent)
---
 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 f8a3ab82e77f..1b8ec3054642 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -166,7 +166,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 0;
 			}
 		}
 		capacity_scale = max(cpu_capacity, capacity_scale);
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* [PATCH v4 4/8] arm: remove wrong CONFIG_PROC_SYSCTL ifdef
  2017-04-20 14:43 ` Juri Lelli
  (?)
@ 2017-04-20 14:43   ` Juri Lelli
  -1 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	juri.lelli, broonie, gregkh

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>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 1b8ec3054642..40dd35aa46d0 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -57,7 +57,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)
@@ -114,7 +113,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	[flat|nested] 63+ messages in thread
* [PATCH v4 4/8] arm: remove wrong CONFIG_PROC_SYSCTL ifdef
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: mark.rutland, devicetree, lorenzo.pieralisi, vincent.guittot,
	juri.lelli, linux-pm, peterz, catalin.marinas, broonie,
	will.deacon, gregkh, dietmar.eggemann, robh+dt, sudeep.holla,
	linux, morten.rasmussen, linux-arm-kernel

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>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 1b8ec3054642..40dd35aa46d0 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -57,7 +57,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)
@@ -114,7 +113,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	[flat|nested] 63+ messages in thread
* [PATCH v4 4/8] arm: remove wrong CONFIG_PROC_SYSCTL ifdef
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-arm-kernel

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>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 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 1b8ec3054642..40dd35aa46d0 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -57,7 +57,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)
@@ -114,7 +113,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	[flat|nested] 63+ messages in thread
* [PATCH v4 5/8] arm, arm64: factorize common cpu capacity default code
  2017-04-20 14:43 ` Juri Lelli
@ 2017-04-20 14:43   ` Juri Lelli
  -1 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	juri.lelli, broonie, gregkh, Russell King

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>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---

Changes from v2:
 - make capacity_scale and raw_capacity static
 - added SPDX header
 - improved indent
 - misc. whitespaces/newlines fixes

Changes from v1:
 - keep the original GPLv2 header
---
 arch/arm/Kconfig             |   1 +
 arch/arm/kernel/topology.c   | 213 ++-----------------------------------
 arch/arm64/Kconfig           |   1 +
 arch/arm64/kernel/topology.c | 219 +--------------------------------------
 drivers/base/Kconfig         |   8 ++
 drivers/base/Makefile        |   1 +
 drivers/base/arch_topology.c | 242 +++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 262 insertions(+), 423 deletions(-)
 create mode 100644 drivers/base/arch_topology.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0d4e71b42c77..cd61154bb6d0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -25,6 +25,7 @@ config ARM
 	select EDAC_SUPPORT
 	select EDAC_ATOMIC_SCRUB
 	select GENERIC_ALLOCATOR
+	select GENERIC_ARCH_TOPOLOGY if ARM_CPU_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 40dd35aa46d0..49ef025ffaa0 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -44,75 +44,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 {
@@ -141,145 +76,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 0;
-			}
-		}
-		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 3741859765cf..e36fb12afad6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -40,6 +40,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 08243533e5ee..c5bc31eb97e8 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -11,7 +11,6 @@
  * for more details.
  */
 
-#include <linux/acpi.h>
 #include <linux/cpu.h>
 #include <linux/cpumask.h>
 #include <linux/init.h>
@@ -23,226 +22,14 @@
 #include <linux/sched/topology.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)
-{
-	/*
-	 * on ACPI-based systems we need to use the default cpu capacity
-	 * until we have the necessary code to parse the cpu capacity, so
-	 * skip registering cpufreq notifier.
-	 */
-	if (!acpi_disabled || 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..f046d21de57d 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..097834f0fcd7
--- /dev/null
+++ b/drivers/base/arch_topology.c
@@ -0,0 +1,242 @@
+/*
+ * Arch specific cpu topology information
+ *
+ * Copyright (C) 2016, ARM Ltd.
+ * Written by: Juri Lelli, ARM Ltd.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <linux/acpi.h>
+#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/sched/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;
+	int i;
+	unsigned long new_capacity;
+	ssize_t ret;
+
+	if (!count)
+		return 0;
+
+	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;
+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 0;
+			}
+		}
+		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)
+{
+	/*
+	 * on ACPI-based systems we need to use the default cpu capacity
+	 * until we have the necessary code to parse the cpu capacity, so
+	 * skip registering cpufreq notifier.
+	 */
+	if (!acpi_disabled || 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	[flat|nested] 63+ messages in thread
* [PATCH v4 5/8] arm, arm64: factorize common cpu capacity default code
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-arm-kernel

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>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---

Changes from v2:
 - make capacity_scale and raw_capacity static
 - added SPDX header
 - improved indent
 - misc. whitespaces/newlines fixes

Changes from v1:
 - keep the original GPLv2 header
---
 arch/arm/Kconfig             |   1 +
 arch/arm/kernel/topology.c   | 213 ++-----------------------------------
 arch/arm64/Kconfig           |   1 +
 arch/arm64/kernel/topology.c | 219 +--------------------------------------
 drivers/base/Kconfig         |   8 ++
 drivers/base/Makefile        |   1 +
 drivers/base/arch_topology.c | 242 +++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 262 insertions(+), 423 deletions(-)
 create mode 100644 drivers/base/arch_topology.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0d4e71b42c77..cd61154bb6d0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -25,6 +25,7 @@ config ARM
 	select EDAC_SUPPORT
 	select EDAC_ATOMIC_SCRUB
 	select GENERIC_ALLOCATOR
+	select GENERIC_ARCH_TOPOLOGY if ARM_CPU_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 40dd35aa46d0..49ef025ffaa0 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -44,75 +44,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 {
@@ -141,145 +76,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 0;
-			}
-		}
-		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 3741859765cf..e36fb12afad6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -40,6 +40,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 08243533e5ee..c5bc31eb97e8 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -11,7 +11,6 @@
  * for more details.
  */
 
-#include <linux/acpi.h>
 #include <linux/cpu.h>
 #include <linux/cpumask.h>
 #include <linux/init.h>
@@ -23,226 +22,14 @@
 #include <linux/sched/topology.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)
-{
-	/*
-	 * on ACPI-based systems we need to use the default cpu capacity
-	 * until we have the necessary code to parse the cpu capacity, so
-	 * skip registering cpufreq notifier.
-	 */
-	if (!acpi_disabled || 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..f046d21de57d 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..097834f0fcd7
--- /dev/null
+++ b/drivers/base/arch_topology.c
@@ -0,0 +1,242 @@
+/*
+ * Arch specific cpu topology information
+ *
+ * Copyright (C) 2016, ARM Ltd.
+ * Written by: Juri Lelli, ARM Ltd.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <linux/acpi.h>
+#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/sched/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;
+	int i;
+	unsigned long new_capacity;
+	ssize_t ret;
+
+	if (!count)
+		return 0;
+
+	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;
+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 0;
+			}
+		}
+		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)
+{
+	/*
+	 * on ACPI-based systems we need to use the default cpu capacity
+	 * until we have the necessary code to parse the cpu capacity, so
+	 * skip registering cpufreq notifier.
+	 */
+	if (!acpi_disabled || 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	[flat|nested] 63+ messages in thread
* [PATCH v4 6/8] arm,arm64,drivers: reduce scope of cap_parsing_failed
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	juri.lelli, broonie, gregkh

Reduce the scope of cap_parsing_failed (making it static in
drivers/base/arch_topology.c) by slightly changing {arm,arm64} DT
parsing code.

For arm checking for !cap_parsing_failed before calling normalize_
cpu_capacity() is superfluous, as returning an error from parse_
cpu_capacity() (above) means cap_from _dt is set to false.

For arm64 we can simply check if raw_capacity points to something,
which is not if capacity parsing has failed.

Suggested-by: Morten Rasmussen <morten.rasmussen@arm.com>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Catalin Marinas <catalin.marinas@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 49ef025ffaa0..1e35a3265ddf 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -76,7 +76,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);
 
@@ -165,7 +164,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 c5bc31eb97e8..7e1f6f75185b 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -27,7 +27,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);
 
@@ -187,10 +186,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 097834f0fcd7..acf99372c5cf 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -95,7 +95,7 @@ subsys_initcall(register_cpu_capacity_sysctl);
 
 static u32 capacity_scale;
 static u32 *raw_capacity;
-bool cap_parsing_failed;
+static bool cap_parsing_failed;
 
 void normalize_cpu_capacity(void)
 {
@@ -210,7 +210,7 @@ static int __init register_cpufreq_notifier(void)
 	 * until we have the necessary code to parse the cpu capacity, so
 	 * skip registering cpufreq notifier.
 	 */
-	if (!acpi_disabled || cap_parsing_failed)
+	if (!acpi_disabled || !raw_capacity)
 		return -EINVAL;
 
 	if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* [PATCH v4 6/8] arm,arm64,drivers: reduce scope of cap_parsing_failed
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-lFZ/pmaqli7XmaaqVzeoHQ, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, morten.rasmussen-5wv7dgnIgG8,
	dietmar.eggemann-5wv7dgnIgG8, juri.lelli-5wv7dgnIgG8,
	broonie-DgEjT+Ai2ygdnm+yROfE0A,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r

Reduce the scope of cap_parsing_failed (making it static in
drivers/base/arch_topology.c) by slightly changing {arm,arm64} DT
parsing code.

For arm checking for !cap_parsing_failed before calling normalize_
cpu_capacity() is superfluous, as returning an error from parse_
cpu_capacity() (above) means cap_from _dt is set to false.

For arm64 we can simply check if raw_capacity points to something,
which is not if capacity parsing has failed.

Suggested-by: Morten Rasmussen <morten.rasmussen-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Juri Lelli <juri.lelli-5wv7dgnIgG8@public.gmane.org>
Acked-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
Acked-by: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
---
 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 49ef025ffaa0..1e35a3265ddf 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -76,7 +76,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);
 
@@ -165,7 +164,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 c5bc31eb97e8..7e1f6f75185b 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -27,7 +27,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);
 
@@ -187,10 +186,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 097834f0fcd7..acf99372c5cf 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -95,7 +95,7 @@ subsys_initcall(register_cpu_capacity_sysctl);
 
 static u32 capacity_scale;
 static u32 *raw_capacity;
-bool cap_parsing_failed;
+static bool cap_parsing_failed;
 
 void normalize_cpu_capacity(void)
 {
@@ -210,7 +210,7 @@ static int __init register_cpufreq_notifier(void)
 	 * until we have the necessary code to parse the cpu capacity, so
 	 * skip registering cpufreq notifier.
 	 */
-	if (!acpi_disabled || cap_parsing_failed)
+	if (!acpi_disabled || !raw_capacity)
 		return -EINVAL;
 
 	if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* [PATCH v4 6/8] arm,arm64,drivers: reduce scope of cap_parsing_failed
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-arm-kernel

Reduce the scope of cap_parsing_failed (making it static in
drivers/base/arch_topology.c) by slightly changing {arm,arm64} DT
parsing code.

For arm checking for !cap_parsing_failed before calling normalize_
cpu_capacity() is superfluous, as returning an error from parse_
cpu_capacity() (above) means cap_from _dt is set to false.

For arm64 we can simply check if raw_capacity points to something,
which is not if capacity parsing has failed.

Suggested-by: Morten Rasmussen <morten.rasmussen@arm.com>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Catalin Marinas <catalin.marinas@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 49ef025ffaa0..1e35a3265ddf 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -76,7 +76,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);
 
@@ -165,7 +164,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 c5bc31eb97e8..7e1f6f75185b 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -27,7 +27,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);
 
@@ -187,10 +186,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 097834f0fcd7..acf99372c5cf 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -95,7 +95,7 @@ subsys_initcall(register_cpu_capacity_sysctl);
 
 static u32 capacity_scale;
 static u32 *raw_capacity;
-bool cap_parsing_failed;
+static bool cap_parsing_failed;
 
 void normalize_cpu_capacity(void)
 {
@@ -210,7 +210,7 @@ static int __init register_cpufreq_notifier(void)
 	 * until we have the necessary code to parse the cpu capacity, so
 	 * skip registering cpufreq notifier.
 	 */
-	if (!acpi_disabled || cap_parsing_failed)
+	if (!acpi_disabled || !raw_capacity)
 		return -EINVAL;
 
 	if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* [PATCH v4 7/8] arm,arm64,drivers: move externs in a new header file
  2017-04-20 14:43 ` Juri Lelli
@ 2017-04-20 14:43   ` Juri Lelli
  -1 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	juri.lelli, broonie, gregkh

Create a new header file (include/linux/arch_topology.h) and put there
declarations of interfaces used by arm, arm64 and drivers code.

Signed-off-by: Juri Lelli <juri.lelli@arm.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/kernel/topology.c    |  7 +------
 arch/arm64/kernel/topology.c  |  4 +---
 drivers/base/arch_topology.c  |  1 +
 include/linux/arch_topology.h | 17 +++++++++++++++++
 4 files changed, 20 insertions(+), 9 deletions(-)
 create mode 100644 include/linux/arch_topology.h

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 1e35a3265ddf..557be4f1d2d7 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -11,6 +11,7 @@
  * for more details.
  */
 
+#include <linux/arch_topology.h>
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/cpumask.h>
@@ -45,10 +46,6 @@
  * updated during this sequence.
  */
 
-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 {
 	const char *compatible;
@@ -76,8 +73,6 @@ static unsigned long *__cpu_capacity;
 
 static unsigned long middle_capacity = 1;
 static bool cap_from_dt = true;
-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/kernel/topology.c b/arch/arm64/kernel/topology.c
index 7e1f6f75185b..255230c3e835 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -11,6 +11,7 @@
  * for more details.
  */
 
+#include <linux/arch_topology.h>
 #include <linux/cpu.h>
 #include <linux/cpumask.h>
 #include <linux/init.h>
@@ -27,9 +28,6 @@
 #include <asm/cputype.h>
 #include <asm/topology.h>
 
-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)
 {
 	struct device_node *cpu_node;
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index acf99372c5cf..76c19aa0d82f 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -13,6 +13,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/arch_topology.h>
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/device.h>
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
new file mode 100644
index 000000000000..4edae9fe8cdd
--- /dev/null
+++ b/include/linux/arch_topology.h
@@ -0,0 +1,17 @@
+/*
+ * include/linux/arch_topology.h - arch specific cpu topology information
+ */
+#ifndef _LINUX_ARCH_TOPOLOGY_H_
+#define _LINUX_ARCH_TOPOLOGY_H_
+
+void normalize_cpu_capacity(void);
+
+struct device_node;
+int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
+
+struct sched_domain;
+unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
+
+void set_capacity_scale(unsigned int cpu, unsigned long capacity);
+
+#endif /* _LINUX_ARCH_TOPOLOGY_H_ */
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* [PATCH v4 7/8] arm,arm64,drivers: move externs in a new header file
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-arm-kernel

Create a new header file (include/linux/arch_topology.h) and put there
declarations of interfaces used by arm, arm64 and drivers code.

Signed-off-by: Juri Lelli <juri.lelli@arm.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/kernel/topology.c    |  7 +------
 arch/arm64/kernel/topology.c  |  4 +---
 drivers/base/arch_topology.c  |  1 +
 include/linux/arch_topology.h | 17 +++++++++++++++++
 4 files changed, 20 insertions(+), 9 deletions(-)
 create mode 100644 include/linux/arch_topology.h

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 1e35a3265ddf..557be4f1d2d7 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -11,6 +11,7 @@
  * for more details.
  */
 
+#include <linux/arch_topology.h>
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/cpumask.h>
@@ -45,10 +46,6 @@
  * updated during this sequence.
  */
 
-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 {
 	const char *compatible;
@@ -76,8 +73,6 @@ static unsigned long *__cpu_capacity;
 
 static unsigned long middle_capacity = 1;
 static bool cap_from_dt = true;
-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/kernel/topology.c b/arch/arm64/kernel/topology.c
index 7e1f6f75185b..255230c3e835 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -11,6 +11,7 @@
  * for more details.
  */
 
+#include <linux/arch_topology.h>
 #include <linux/cpu.h>
 #include <linux/cpumask.h>
 #include <linux/init.h>
@@ -27,9 +28,6 @@
 #include <asm/cputype.h>
 #include <asm/topology.h>
 
-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)
 {
 	struct device_node *cpu_node;
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index acf99372c5cf..76c19aa0d82f 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -13,6 +13,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/arch_topology.h>
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/device.h>
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
new file mode 100644
index 000000000000..4edae9fe8cdd
--- /dev/null
+++ b/include/linux/arch_topology.h
@@ -0,0 +1,17 @@
+/*
+ * include/linux/arch_topology.h - arch specific cpu topology information
+ */
+#ifndef _LINUX_ARCH_TOPOLOGY_H_
+#define _LINUX_ARCH_TOPOLOGY_H_
+
+void normalize_cpu_capacity(void);
+
+struct device_node;
+int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
+
+struct sched_domain;
+unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
+
+void set_capacity_scale(unsigned int cpu, unsigned long capacity);
+
+#endif /* _LINUX_ARCH_TOPOLOGY_H_ */
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
  2017-04-20 14:43 ` Juri Lelli
@ 2017-04-20 14:43   ` Juri Lelli
  -1 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	juri.lelli, broonie, gregkh

Now that some functions that deal with arch topology information live
under drivers, there is a clash of naming that might create confusion.

Tidy things up by creating a drivers namespace for interfaces used by
arch code; achieve this by prepending a 'atd_' (arch topology driver)
prefix to driver interfaces.

Signed-off-by: Juri Lelli <juri.lelli@arm.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/kernel/topology.c    |  8 ++++----
 arch/arm64/kernel/topology.c  |  4 ++--
 drivers/base/arch_topology.c  | 20 ++++++++++----------
 include/linux/arch_topology.h |  8 ++++----
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 557be4f1d2d7..e53391026c1b 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -111,7 +111,7 @@ static void __init parse_dt_topology(void)
 			continue;
 		}
 
-		if (parse_cpu_capacity(cn, cpu)) {
+		if (atd_parse_cpu_capacity(cn, cpu)) {
 			of_node_put(cn);
 			continue;
 		}
@@ -160,7 +160,7 @@ static void __init parse_dt_topology(void)
 				>> (SCHED_CAPACITY_SHIFT-1)) + 1;
 
 	if (cap_from_dt)
-		normalize_cpu_capacity();
+		atd_normalize_cpu_capacity();
 }
 
 /*
@@ -173,10 +173,10 @@ static void update_cpu_capacity(unsigned int cpu)
 	if (!cpu_capacity(cpu) || cap_from_dt)
 		return;
 
-	set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+	atd_set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
 
 	pr_info("CPU%u: update cpu_capacity %lu\n",
-		cpu, arch_scale_cpu_capacity(NULL, cpu));
+		cpu, atd_scale_cpu_capacity(NULL, cpu));
 }
 
 #else
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 255230c3e835..5f24faa09c05 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -39,7 +39,7 @@ static int __init get_cpu_for_node(struct device_node *node)
 
 	for_each_possible_cpu(cpu) {
 		if (of_get_cpu_node(cpu, NULL) == cpu_node) {
-			parse_cpu_capacity(cpu_node, cpu);
+			atd_parse_cpu_capacity(cpu_node, cpu);
 			of_node_put(cpu_node);
 			return cpu;
 		}
@@ -191,7 +191,7 @@ static int __init parse_dt_topology(void)
 	if (ret != 0)
 		goto out_map;
 
-	normalize_cpu_capacity();
+	atd_normalize_cpu_capacity();
 
 	/*
 	 * Check that all cores are in the topology; the SMP code will
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 76c19aa0d82f..f04999e3ff75 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -25,12 +25,12 @@
 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)
+unsigned long atd_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)
+void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity)
 {
 	per_cpu(cpu_scale, cpu) = capacity;
 }
@@ -42,7 +42,7 @@ static ssize_t cpu_capacity_show(struct device *dev,
 	struct cpu *cpu = container_of(dev, struct cpu, dev);
 
 	return sprintf(buf, "%lu\n",
-			arch_scale_cpu_capacity(NULL, cpu->dev.id));
+			atd_scale_cpu_capacity(NULL, cpu->dev.id));
 }
 
 static ssize_t cpu_capacity_store(struct device *dev,
@@ -67,7 +67,7 @@ static ssize_t cpu_capacity_store(struct device *dev,
 
 	mutex_lock(&cpu_scale_mutex);
 	for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
-		set_capacity_scale(i, new_capacity);
+		atd_set_capacity_scale(i, new_capacity);
 	mutex_unlock(&cpu_scale_mutex);
 
 	return count;
@@ -98,7 +98,7 @@ static u32 capacity_scale;
 static u32 *raw_capacity;
 static bool cap_parsing_failed;
 
-void normalize_cpu_capacity(void)
+void atd_normalize_cpu_capacity(void)
 {
 	u64 capacity;
 	int cpu;
@@ -113,14 +113,14 @@ void normalize_cpu_capacity(void)
 			 cpu, raw_capacity[cpu]);
 		capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
 			/ capacity_scale;
-		set_capacity_scale(cpu, capacity);
+		atd_set_capacity_scale(cpu, capacity);
 		pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
-			cpu, arch_scale_cpu_capacity(NULL, cpu));
+			cpu, atd_scale_cpu_capacity(NULL, cpu));
 	}
 	mutex_unlock(&cpu_scale_mutex);
 }
 
-int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
+int __init atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
 {
 	int ret = 1;
 	u32 cpu_capacity;
@@ -185,12 +185,12 @@ init_cpu_capacity_callback(struct notifier_block *nb,
 			       cpus_to_visit,
 			       policy->related_cpus);
 		for_each_cpu(cpu, policy->related_cpus) {
-			raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
+			raw_capacity[cpu] = atd_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();
+			atd_normalize_cpu_capacity();
 			kfree(raw_capacity);
 			pr_debug("cpu_capacity: parsing done\n");
 			cap_parsing_done = true;
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index 4edae9fe8cdd..e25458d7ee9a 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -4,14 +4,14 @@
 #ifndef _LINUX_ARCH_TOPOLOGY_H_
 #define _LINUX_ARCH_TOPOLOGY_H_
 
-void normalize_cpu_capacity(void);
+void atd_normalize_cpu_capacity(void);
 
 struct device_node;
-int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
+int atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
 
 struct sched_domain;
-unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
+unsigned long atd_scale_cpu_capacity(struct sched_domain *sd, int cpu);
 
-void set_capacity_scale(unsigned int cpu, unsigned long capacity);
+void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity);
 
 #endif /* _LINUX_ARCH_TOPOLOGY_H_ */
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* [PATCH v4 8/8] arm, arm64, drivers: add a prefix to drivers arch_topology interfaces
@ 2017-04-20 14:43   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-04-20 14:43 UTC (permalink / raw)
  To: linux-arm-kernel

Now that some functions that deal with arch topology information live
under drivers, there is a clash of naming that might create confusion.

Tidy things up by creating a drivers namespace for interfaces used by
arch code; achieve this by prepending a 'atd_' (arch topology driver)
prefix to driver interfaces.

Signed-off-by: Juri Lelli <juri.lelli@arm.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/kernel/topology.c    |  8 ++++----
 arch/arm64/kernel/topology.c  |  4 ++--
 drivers/base/arch_topology.c  | 20 ++++++++++----------
 include/linux/arch_topology.h |  8 ++++----
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 557be4f1d2d7..e53391026c1b 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -111,7 +111,7 @@ static void __init parse_dt_topology(void)
 			continue;
 		}
 
-		if (parse_cpu_capacity(cn, cpu)) {
+		if (atd_parse_cpu_capacity(cn, cpu)) {
 			of_node_put(cn);
 			continue;
 		}
@@ -160,7 +160,7 @@ static void __init parse_dt_topology(void)
 				>> (SCHED_CAPACITY_SHIFT-1)) + 1;
 
 	if (cap_from_dt)
-		normalize_cpu_capacity();
+		atd_normalize_cpu_capacity();
 }
 
 /*
@@ -173,10 +173,10 @@ static void update_cpu_capacity(unsigned int cpu)
 	if (!cpu_capacity(cpu) || cap_from_dt)
 		return;
 
-	set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+	atd_set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
 
 	pr_info("CPU%u: update cpu_capacity %lu\n",
-		cpu, arch_scale_cpu_capacity(NULL, cpu));
+		cpu, atd_scale_cpu_capacity(NULL, cpu));
 }
 
 #else
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 255230c3e835..5f24faa09c05 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -39,7 +39,7 @@ static int __init get_cpu_for_node(struct device_node *node)
 
 	for_each_possible_cpu(cpu) {
 		if (of_get_cpu_node(cpu, NULL) == cpu_node) {
-			parse_cpu_capacity(cpu_node, cpu);
+			atd_parse_cpu_capacity(cpu_node, cpu);
 			of_node_put(cpu_node);
 			return cpu;
 		}
@@ -191,7 +191,7 @@ static int __init parse_dt_topology(void)
 	if (ret != 0)
 		goto out_map;
 
-	normalize_cpu_capacity();
+	atd_normalize_cpu_capacity();
 
 	/*
 	 * Check that all cores are in the topology; the SMP code will
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 76c19aa0d82f..f04999e3ff75 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -25,12 +25,12 @@
 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)
+unsigned long atd_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)
+void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity)
 {
 	per_cpu(cpu_scale, cpu) = capacity;
 }
@@ -42,7 +42,7 @@ static ssize_t cpu_capacity_show(struct device *dev,
 	struct cpu *cpu = container_of(dev, struct cpu, dev);
 
 	return sprintf(buf, "%lu\n",
-			arch_scale_cpu_capacity(NULL, cpu->dev.id));
+			atd_scale_cpu_capacity(NULL, cpu->dev.id));
 }
 
 static ssize_t cpu_capacity_store(struct device *dev,
@@ -67,7 +67,7 @@ static ssize_t cpu_capacity_store(struct device *dev,
 
 	mutex_lock(&cpu_scale_mutex);
 	for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
-		set_capacity_scale(i, new_capacity);
+		atd_set_capacity_scale(i, new_capacity);
 	mutex_unlock(&cpu_scale_mutex);
 
 	return count;
@@ -98,7 +98,7 @@ static u32 capacity_scale;
 static u32 *raw_capacity;
 static bool cap_parsing_failed;
 
-void normalize_cpu_capacity(void)
+void atd_normalize_cpu_capacity(void)
 {
 	u64 capacity;
 	int cpu;
@@ -113,14 +113,14 @@ void normalize_cpu_capacity(void)
 			 cpu, raw_capacity[cpu]);
 		capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
 			/ capacity_scale;
-		set_capacity_scale(cpu, capacity);
+		atd_set_capacity_scale(cpu, capacity);
 		pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
-			cpu, arch_scale_cpu_capacity(NULL, cpu));
+			cpu, atd_scale_cpu_capacity(NULL, cpu));
 	}
 	mutex_unlock(&cpu_scale_mutex);
 }
 
-int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
+int __init atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
 {
 	int ret = 1;
 	u32 cpu_capacity;
@@ -185,12 +185,12 @@ init_cpu_capacity_callback(struct notifier_block *nb,
 			       cpus_to_visit,
 			       policy->related_cpus);
 		for_each_cpu(cpu, policy->related_cpus) {
-			raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
+			raw_capacity[cpu] = atd_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();
+			atd_normalize_cpu_capacity();
 			kfree(raw_capacity);
 			pr_debug("cpu_capacity: parsing done\n");
 			cap_parsing_done = true;
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index 4edae9fe8cdd..e25458d7ee9a 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -4,14 +4,14 @@
 #ifndef _LINUX_ARCH_TOPOLOGY_H_
 #define _LINUX_ARCH_TOPOLOGY_H_
 
-void normalize_cpu_capacity(void);
+void atd_normalize_cpu_capacity(void);
 
 struct device_node;
-int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
+int atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
 
 struct sched_domain;
-unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
+unsigned long atd_scale_cpu_capacity(struct sched_domain *sd, int cpu);
 
-void set_capacity_scale(unsigned int cpu, unsigned long capacity);
+void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity);
 
 #endif /* _LINUX_ARCH_TOPOLOGY_H_ */
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 3/8] arm: fix return value of parse_cpu_capacity
  2017-04-20 14:43   ` Juri Lelli
@ 2017-04-20 14:50     ` Vincent Guittot
  -1 siblings, 0 replies; 63+ messages in thread
From: Vincent Guittot @ 2017-04-20 14:50 UTC (permalink / raw)
  To: Juri Lelli
  Cc: linux-kernel, linux-pm, LAK, devicetree, Peter Zijlstra,
	Rob Herring, Mark Rutland, Russell King - ARM Linux,
	Sudeep Holla, Lorenzo Pieralisi, Catalin Marinas, Will Deacon,
	Morten Rasmussen, Dietmar Eggemann, Mark Brown, gregkh

On 20 April 2017 at 16:43, Juri Lelli <juri.lelli@arm.com> wrote:
> parse_cpu_capacity() has to return 0 on failure, but it currently returns
> 1 instead if raw_capacity kcalloc failed.
>
> Fix it (by directly returning 0).
>
> 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>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>
> Changes from v3:
>
>  - directly return 0 on failure (as pointed out by Vincent)
> ---
>  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 f8a3ab82e77f..1b8ec3054642 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -166,7 +166,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 0;

Acked-by: Vincent Guittot <vincent.guittot@linaor.org>
>                         }
>                 }
>                 capacity_scale = max(cpu_capacity, capacity_scale);
> --
> 2.10.0
>

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 3/8] arm: fix return value of parse_cpu_capacity
@ 2017-04-20 14:50     ` Vincent Guittot
  0 siblings, 0 replies; 63+ messages in thread
From: Vincent Guittot @ 2017-04-20 14:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 20 April 2017 at 16:43, Juri Lelli <juri.lelli@arm.com> wrote:
> parse_cpu_capacity() has to return 0 on failure, but it currently returns
> 1 instead if raw_capacity kcalloc failed.
>
> Fix it (by directly returning 0).
>
> 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>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>
> Changes from v3:
>
>  - directly return 0 on failure (as pointed out by Vincent)
> ---
>  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 f8a3ab82e77f..1b8ec3054642 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -166,7 +166,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 0;

Acked-by: Vincent Guittot <vincent.guittot@linaor.org>
>                         }
>                 }
>                 capacity_scale = max(cpu_capacity, capacity_scale);
> --
> 2.10.0
>

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-11  8:48   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-11  8:48 UTC (permalink / raw)
  To: gregkh
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	broonie, linux-kernel

Hi Greg,

I'm not sure what your plans are with this set. Are we targeting 4.12?

IMHO, if possible, it would be good to have this refactoring in now, so
that we can base further development on it for the next cycle.

Thanks,

- Juri

On 20/04/17 15:43, Juri Lelli wrote:
> Hi,
> 
> arm and arm64 topology.c share a lot of code related to parsing of capacity
> information. This is v4 of a solution [1] (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 4 patches are actually fixes for the current code.
> 
> Patch 5 is the actual refactoring.
> 
> Patch 6 removes one of the extern symbols by changing a bit the now common
> code.
> 
> Patch 7 removes the remaining externs (as required by Russell during v1 review)
> by creating a new header file include/linux/arch_topology.h and including that
> from arm, arm64 and drivers.
> 
> Last patch addresses Dietmar's comments to v1 and adds a 'atd_' prefix to
> interfaces exported by drivers code and used by arch (and potentially others in
> the future).
> 
> Changes from v3:
> 
>  - rebase on top of 4.11-rc7
>  - addressed Vincent's comment on patch 3/8
>  - removed v3 6/9 as discussed with Russell
>  - added Russell and Catalin's Acked-by
> 
> The set is based on top of linux/master (4.11-rc7 f61143c45077) and it is also
> available from:
> 
>  git://linux-arm.org/linux-jl.git upstream/default_caps_factorize-v4
> 
> Best,
> 
> - Juri
> 
> [1] v1 - https://marc.info/?l=linux-kernel&m=148483680119355&w=2
>     v2 - https://marc.info/?l=linux-kernel&m=148663344018205&w=2
>     v3 - http://marc.info/?l=linux-kernel&m=149062080701399&w=2
> 
> Juri Lelli (8):
>   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
>   arm, arm64: factorize common cpu capacity default code
>   arm,arm64,drivers: reduce scope of cap_parsing_failed
>   arm,arm64,drivers: move externs in a new header file
>   arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
> 
>  Documentation/ABI/testing/sysfs-devices-system-cpu |   7 +
>  Documentation/devicetree/bindings/arm/cpus.txt     |   4 +-
>  arch/arm/Kconfig                                   |   1 +
>  arch/arm/kernel/topology.c                         | 221 +------------------
>  arch/arm64/Kconfig                                 |   1 +
>  arch/arm64/kernel/topology.c                       | 226 +------------------
>  drivers/base/Kconfig                               |   8 +
>  drivers/base/Makefile                              |   1 +
>  drivers/base/arch_topology.c                       | 243 +++++++++++++++++++++
>  include/linux/arch_topology.h                      |  17 ++
>  10 files changed, 290 insertions(+), 439 deletions(-)
>  create mode 100644 drivers/base/arch_topology.c
>  create mode 100644 include/linux/arch_topology.h
> 
> -- 
> 2.10.0
> 

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-11  8:48   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-11  8:48 UTC (permalink / raw)
  To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-lFZ/pmaqli7XmaaqVzeoHQ, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, morten.rasmussen-5wv7dgnIgG8,
	dietmar.eggemann-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi Greg,

I'm not sure what your plans are with this set. Are we targeting 4.12?

IMHO, if possible, it would be good to have this refactoring in now, so
that we can base further development on it for the next cycle.

Thanks,

- Juri

On 20/04/17 15:43, Juri Lelli wrote:
> Hi,
> 
> arm and arm64 topology.c share a lot of code related to parsing of capacity
> information. This is v4 of a solution [1] (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 4 patches are actually fixes for the current code.
> 
> Patch 5 is the actual refactoring.
> 
> Patch 6 removes one of the extern symbols by changing a bit the now common
> code.
> 
> Patch 7 removes the remaining externs (as required by Russell during v1 review)
> by creating a new header file include/linux/arch_topology.h and including that
> from arm, arm64 and drivers.
> 
> Last patch addresses Dietmar's comments to v1 and adds a 'atd_' prefix to
> interfaces exported by drivers code and used by arch (and potentially others in
> the future).
> 
> Changes from v3:
> 
>  - rebase on top of 4.11-rc7
>  - addressed Vincent's comment on patch 3/8
>  - removed v3 6/9 as discussed with Russell
>  - added Russell and Catalin's Acked-by
> 
> The set is based on top of linux/master (4.11-rc7 f61143c45077) and it is also
> available from:
> 
>  git://linux-arm.org/linux-jl.git upstream/default_caps_factorize-v4
> 
> Best,
> 
> - Juri
> 
> [1] v1 - https://marc.info/?l=linux-kernel&m=148483680119355&w=2
>     v2 - https://marc.info/?l=linux-kernel&m=148663344018205&w=2
>     v3 - http://marc.info/?l=linux-kernel&m=149062080701399&w=2
> 
> Juri Lelli (8):
>   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
>   arm, arm64: factorize common cpu capacity default code
>   arm,arm64,drivers: reduce scope of cap_parsing_failed
>   arm,arm64,drivers: move externs in a new header file
>   arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
> 
>  Documentation/ABI/testing/sysfs-devices-system-cpu |   7 +
>  Documentation/devicetree/bindings/arm/cpus.txt     |   4 +-
>  arch/arm/Kconfig                                   |   1 +
>  arch/arm/kernel/topology.c                         | 221 +------------------
>  arch/arm64/Kconfig                                 |   1 +
>  arch/arm64/kernel/topology.c                       | 226 +------------------
>  drivers/base/Kconfig                               |   8 +
>  drivers/base/Makefile                              |   1 +
>  drivers/base/arch_topology.c                       | 243 +++++++++++++++++++++
>  include/linux/arch_topology.h                      |  17 ++
>  10 files changed, 290 insertions(+), 439 deletions(-)
>  create mode 100644 drivers/base/arch_topology.c
>  create mode 100644 include/linux/arch_topology.h
> 
> -- 
> 2.10.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-11  8:48   ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-11  8:48 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Greg,

I'm not sure what your plans are with this set. Are we targeting 4.12?

IMHO, if possible, it would be good to have this refactoring in now, so
that we can base further development on it for the next cycle.

Thanks,

- Juri

On 20/04/17 15:43, Juri Lelli wrote:
> Hi,
> 
> arm and arm64 topology.c share a lot of code related to parsing of capacity
> information. This is v4 of a solution [1] (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 4 patches are actually fixes for the current code.
> 
> Patch 5 is the actual refactoring.
> 
> Patch 6 removes one of the extern symbols by changing a bit the now common
> code.
> 
> Patch 7 removes the remaining externs (as required by Russell during v1 review)
> by creating a new header file include/linux/arch_topology.h and including that
> from arm, arm64 and drivers.
> 
> Last patch addresses Dietmar's comments to v1 and adds a 'atd_' prefix to
> interfaces exported by drivers code and used by arch (and potentially others in
> the future).
> 
> Changes from v3:
> 
>  - rebase on top of 4.11-rc7
>  - addressed Vincent's comment on patch 3/8
>  - removed v3 6/9 as discussed with Russell
>  - added Russell and Catalin's Acked-by
> 
> The set is based on top of linux/master (4.11-rc7 f61143c45077) and it is also
> available from:
> 
>  git://linux-arm.org/linux-jl.git upstream/default_caps_factorize-v4
> 
> Best,
> 
> - Juri
> 
> [1] v1 - https://marc.info/?l=linux-kernel&m=148483680119355&w=2
>     v2 - https://marc.info/?l=linux-kernel&m=148663344018205&w=2
>     v3 - http://marc.info/?l=linux-kernel&m=149062080701399&w=2
> 
> Juri Lelli (8):
>   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
>   arm, arm64: factorize common cpu capacity default code
>   arm,arm64,drivers: reduce scope of cap_parsing_failed
>   arm,arm64,drivers: move externs in a new header file
>   arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
> 
>  Documentation/ABI/testing/sysfs-devices-system-cpu |   7 +
>  Documentation/devicetree/bindings/arm/cpus.txt     |   4 +-
>  arch/arm/Kconfig                                   |   1 +
>  arch/arm/kernel/topology.c                         | 221 +------------------
>  arch/arm64/Kconfig                                 |   1 +
>  arch/arm64/kernel/topology.c                       | 226 +------------------
>  drivers/base/Kconfig                               |   8 +
>  drivers/base/Makefile                              |   1 +
>  drivers/base/arch_topology.c                       | 243 +++++++++++++++++++++
>  include/linux/arch_topology.h                      |  17 ++
>  10 files changed, 290 insertions(+), 439 deletions(-)
>  create mode 100644 drivers/base/arch_topology.c
>  create mode 100644 include/linux/arch_topology.h
> 
> -- 
> 2.10.0
> 

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
  2017-05-11  8:48   ` Juri Lelli
@ 2017-05-11  8:59     ` Greg KH
  -1 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-11  8:59 UTC (permalink / raw)
  To: Juri Lelli
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	broonie, linux-kernel

On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> Hi Greg,
> 
> I'm not sure what your plans are with this set. Are we targeting 4.12?
> 
> IMHO, if possible, it would be good to have this refactoring in now, so
> that we can base further development on it for the next cycle.

It's way too late for new features that have not been in linux-next for
a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
patchset after -rc1 is out, and I get a chance to catch up on pending
patches.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-11  8:59     ` Greg KH
  0 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-11  8:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> Hi Greg,
> 
> I'm not sure what your plans are with this set. Are we targeting 4.12?
> 
> IMHO, if possible, it would be good to have this refactoring in now, so
> that we can base further development on it for the next cycle.

It's way too late for new features that have not been in linux-next for
a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
patchset after -rc1 is out, and I get a chance to catch up on pending
patches.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-11 10:27       ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-11 10:27 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	broonie, linux-kernel

On 11/05/17 10:59, Greg KH wrote:
> On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> > Hi Greg,
> > 
> > I'm not sure what your plans are with this set. Are we targeting 4.12?
> > 
> > IMHO, if possible, it would be good to have this refactoring in now, so
> > that we can base further development on it for the next cycle.
> 
> It's way too late for new features that have not been in linux-next for
> a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
> patchset after -rc1 is out, and I get a chance to catch up on pending
> patches.
> 

Thanks for your reply, I perfecly understand.

OK, let's target 4.13 then. Looking forward to receive your comments
after -rc1 is out.

Best,

- Juri

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-11 10:27       ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-11 10:27 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-lFZ/pmaqli7XmaaqVzeoHQ, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, morten.rasmussen-5wv7dgnIgG8,
	dietmar.eggemann-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 11/05/17 10:59, Greg KH wrote:
> On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> > Hi Greg,
> > 
> > I'm not sure what your plans are with this set. Are we targeting 4.12?
> > 
> > IMHO, if possible, it would be good to have this refactoring in now, so
> > that we can base further development on it for the next cycle.
> 
> It's way too late for new features that have not been in linux-next for
> a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
> patchset after -rc1 is out, and I get a chance to catch up on pending
> patches.
> 

Thanks for your reply, I perfecly understand.

OK, let's target 4.13 then. Looking forward to receive your comments
after -rc1 is out.

Best,

- Juri
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-11 10:27       ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-11 10:27 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/05/17 10:59, Greg KH wrote:
> On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> > Hi Greg,
> > 
> > I'm not sure what your plans are with this set. Are we targeting 4.12?
> > 
> > IMHO, if possible, it would be good to have this refactoring in now, so
> > that we can base further development on it for the next cycle.
> 
> It's way too late for new features that have not been in linux-next for
> a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
> patchset after -rc1 is out, and I get a chance to catch up on pending
> patches.
> 

Thanks for your reply, I perfecly understand.

OK, let's target 4.13 then. Looking forward to receive your comments
after -rc1 is out.

Best,

- Juri

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
  2017-05-11 10:27       ` Juri Lelli
@ 2017-05-24 14:45         ` Juri Lelli
  -1 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-24 14:45 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	broonie, linux-kernel

Hi,

On 11/05/17 11:27, Juri Lelli wrote:
> On 11/05/17 10:59, Greg KH wrote:
> > On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> > > Hi Greg,
> > > 
> > > I'm not sure what your plans are with this set. Are we targeting 4.12?
> > > 
> > > IMHO, if possible, it would be good to have this refactoring in now, so
> > > that we can base further development on it for the next cycle.
> > 
> > It's way too late for new features that have not been in linux-next for
> > a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
> > patchset after -rc1 is out, and I get a chance to catch up on pending
> > patches.
> > 
> 
> Thanks for your reply, I perfecly understand.
> 
> OK, let's target 4.13 then. Looking forward to receive your comments
> after -rc1 is out.
> 

Did you have any chance to have a look at this set?

Thanks,

- Juri

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-24 14:45         ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-24 14:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 11/05/17 11:27, Juri Lelli wrote:
> On 11/05/17 10:59, Greg KH wrote:
> > On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> > > Hi Greg,
> > > 
> > > I'm not sure what your plans are with this set. Are we targeting 4.12?
> > > 
> > > IMHO, if possible, it would be good to have this refactoring in now, so
> > > that we can base further development on it for the next cycle.
> > 
> > It's way too late for new features that have not been in linux-next for
> > a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
> > patchset after -rc1 is out, and I get a chance to catch up on pending
> > patches.
> > 
> 
> Thanks for your reply, I perfecly understand.
> 
> OK, let's target 4.13 then. Looking forward to receive your comments
> after -rc1 is out.
> 

Did you have any chance to have a look at this set?

Thanks,

- Juri

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 5/8] arm, arm64: factorize common cpu capacity default code
  2017-04-20 14:43   ` Juri Lelli
@ 2017-05-25 13:12     ` Greg KH
  -1 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-25 13:12 UTC (permalink / raw)
  To: Juri Lelli
  Cc: linux-kernel, linux-pm, linux-arm-kernel, devicetree, peterz,
	vincent.guittot, robh+dt, mark.rutland, linux, sudeep.holla,
	lorenzo.pieralisi, catalin.marinas, will.deacon,
	morten.rasmussen, dietmar.eggemann, broonie, Russell King

On Thu, Apr 20, 2017 at 03:43:13PM +0100, Juri Lelli wrote:
> 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>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 5/8] arm, arm64: factorize common cpu capacity default code
@ 2017-05-25 13:12     ` Greg KH
  0 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-25 13:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 20, 2017 at 03:43:13PM +0100, Juri Lelli wrote:
> 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>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 6/8] arm,arm64,drivers: reduce scope of cap_parsing_failed
  2017-04-20 14:43   ` Juri Lelli
@ 2017-05-25 13:13     ` Greg KH
  -1 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-25 13:13 UTC (permalink / raw)
  To: Juri Lelli
  Cc: linux-kernel, linux-pm, linux-arm-kernel, devicetree, peterz,
	vincent.guittot, robh+dt, mark.rutland, linux, sudeep.holla,
	lorenzo.pieralisi, catalin.marinas, will.deacon,
	morten.rasmussen, dietmar.eggemann, broonie

On Thu, Apr 20, 2017 at 03:43:14PM +0100, Juri Lelli wrote:
> Reduce the scope of cap_parsing_failed (making it static in
> drivers/base/arch_topology.c) by slightly changing {arm,arm64} DT
> parsing code.
> 
> For arm checking for !cap_parsing_failed before calling normalize_
> cpu_capacity() is superfluous, as returning an error from parse_
> cpu_capacity() (above) means cap_from _dt is set to false.
> 
> For arm64 we can simply check if raw_capacity points to something,
> which is not if capacity parsing has failed.
> 
> Suggested-by: Morten Rasmussen <morten.rasmussen@arm.com>
> Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>


Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 6/8] arm,arm64,drivers: reduce scope of cap_parsing_failed
@ 2017-05-25 13:13     ` Greg KH
  0 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-25 13:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 20, 2017 at 03:43:14PM +0100, Juri Lelli wrote:
> Reduce the scope of cap_parsing_failed (making it static in
> drivers/base/arch_topology.c) by slightly changing {arm,arm64} DT
> parsing code.
> 
> For arm checking for !cap_parsing_failed before calling normalize_
> cpu_capacity() is superfluous, as returning an error from parse_
> cpu_capacity() (above) means cap_from _dt is set to false.
> 
> For arm64 we can simply check if raw_capacity points to something,
> which is not if capacity parsing has failed.
> 
> Suggested-by: Morten Rasmussen <morten.rasmussen@arm.com>
> Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>


Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 7/8] arm,arm64,drivers: move externs in a new header file
  2017-04-20 14:43   ` Juri Lelli
@ 2017-05-25 13:13     ` Greg KH
  -1 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-25 13:13 UTC (permalink / raw)
  To: Juri Lelli
  Cc: linux-kernel, linux-pm, linux-arm-kernel, devicetree, peterz,
	vincent.guittot, robh+dt, mark.rutland, linux, sudeep.holla,
	lorenzo.pieralisi, catalin.marinas, will.deacon,
	morten.rasmussen, dietmar.eggemann, broonie

On Thu, Apr 20, 2017 at 03:43:15PM +0100, Juri Lelli wrote:
> Create a new header file (include/linux/arch_topology.h) and put there
> declarations of interfaces used by arm, arm64 and drivers code.
> 
> Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  arch/arm/kernel/topology.c    |  7 +------
>  arch/arm64/kernel/topology.c  |  4 +---
>  drivers/base/arch_topology.c  |  1 +
>  include/linux/arch_topology.h | 17 +++++++++++++++++
>  4 files changed, 20 insertions(+), 9 deletions(-)
>  create mode 100644 include/linux/arch_topology.h
> 
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index 1e35a3265ddf..557be4f1d2d7 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -11,6 +11,7 @@
>   * for more details.
>   */
>  
> +#include <linux/arch_topology.h>
>  #include <linux/cpu.h>
>  #include <linux/cpufreq.h>
>  #include <linux/cpumask.h>
> @@ -45,10 +46,6 @@
>   * updated during this sequence.
>   */
>  
> -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 {
>  	const char *compatible;
> @@ -76,8 +73,6 @@ static unsigned long *__cpu_capacity;
>  
>  static unsigned long middle_capacity = 1;
>  static bool cap_from_dt = true;
> -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/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 7e1f6f75185b..255230c3e835 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -11,6 +11,7 @@
>   * for more details.
>   */
>  
> +#include <linux/arch_topology.h>
>  #include <linux/cpu.h>
>  #include <linux/cpumask.h>
>  #include <linux/init.h>
> @@ -27,9 +28,6 @@
>  #include <asm/cputype.h>
>  #include <asm/topology.h>
>  
> -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)
>  {
>  	struct device_node *cpu_node;
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index acf99372c5cf..76c19aa0d82f 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -13,6 +13,7 @@
>   */
>  
>  #include <linux/acpi.h>
> +#include <linux/arch_topology.h>
>  #include <linux/cpu.h>
>  #include <linux/cpufreq.h>
>  #include <linux/device.h>
> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> new file mode 100644
> index 000000000000..4edae9fe8cdd
> --- /dev/null
> +++ b/include/linux/arch_topology.h
> @@ -0,0 +1,17 @@
> +/*
> + * include/linux/arch_topology.h - arch specific cpu topology information
> + */
> +#ifndef _LINUX_ARCH_TOPOLOGY_H_
> +#define _LINUX_ARCH_TOPOLOGY_H_
> +
> +void normalize_cpu_capacity(void);
> +
> +struct device_node;
> +int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> +
> +struct sched_domain;
> +unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> +
> +void set_capacity_scale(unsigned int cpu, unsigned long capacity);

Ugh, horrid names, but I'll get to that on your next patch, when you
rename these things...

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 7/8] arm,arm64,drivers: move externs in a new header file
@ 2017-05-25 13:13     ` Greg KH
  0 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-25 13:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 20, 2017 at 03:43:15PM +0100, Juri Lelli wrote:
> Create a new header file (include/linux/arch_topology.h) and put there
> declarations of interfaces used by arm, arm64 and drivers code.
> 
> Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  arch/arm/kernel/topology.c    |  7 +------
>  arch/arm64/kernel/topology.c  |  4 +---
>  drivers/base/arch_topology.c  |  1 +
>  include/linux/arch_topology.h | 17 +++++++++++++++++
>  4 files changed, 20 insertions(+), 9 deletions(-)
>  create mode 100644 include/linux/arch_topology.h
> 
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index 1e35a3265ddf..557be4f1d2d7 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -11,6 +11,7 @@
>   * for more details.
>   */
>  
> +#include <linux/arch_topology.h>
>  #include <linux/cpu.h>
>  #include <linux/cpufreq.h>
>  #include <linux/cpumask.h>
> @@ -45,10 +46,6 @@
>   * updated during this sequence.
>   */
>  
> -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 {
>  	const char *compatible;
> @@ -76,8 +73,6 @@ static unsigned long *__cpu_capacity;
>  
>  static unsigned long middle_capacity = 1;
>  static bool cap_from_dt = true;
> -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/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 7e1f6f75185b..255230c3e835 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -11,6 +11,7 @@
>   * for more details.
>   */
>  
> +#include <linux/arch_topology.h>
>  #include <linux/cpu.h>
>  #include <linux/cpumask.h>
>  #include <linux/init.h>
> @@ -27,9 +28,6 @@
>  #include <asm/cputype.h>
>  #include <asm/topology.h>
>  
> -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)
>  {
>  	struct device_node *cpu_node;
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index acf99372c5cf..76c19aa0d82f 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -13,6 +13,7 @@
>   */
>  
>  #include <linux/acpi.h>
> +#include <linux/arch_topology.h>
>  #include <linux/cpu.h>
>  #include <linux/cpufreq.h>
>  #include <linux/device.h>
> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> new file mode 100644
> index 000000000000..4edae9fe8cdd
> --- /dev/null
> +++ b/include/linux/arch_topology.h
> @@ -0,0 +1,17 @@
> +/*
> + * include/linux/arch_topology.h - arch specific cpu topology information
> + */
> +#ifndef _LINUX_ARCH_TOPOLOGY_H_
> +#define _LINUX_ARCH_TOPOLOGY_H_
> +
> +void normalize_cpu_capacity(void);
> +
> +struct device_node;
> +int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> +
> +struct sched_domain;
> +unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> +
> +void set_capacity_scale(unsigned int cpu, unsigned long capacity);

Ugh, horrid names, but I'll get to that on your next patch, when you
rename these things...

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
  2017-04-20 14:43   ` [PATCH v4 8/8] arm, arm64, drivers: " Juri Lelli
@ 2017-05-25 13:18     ` Greg KH
  -1 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-25 13:18 UTC (permalink / raw)
  To: Juri Lelli
  Cc: linux-kernel, linux-pm, linux-arm-kernel, devicetree, peterz,
	vincent.guittot, robh+dt, mark.rutland, linux, sudeep.holla,
	lorenzo.pieralisi, catalin.marinas, will.deacon,
	morten.rasmussen, dietmar.eggemann, broonie

On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> Now that some functions that deal with arch topology information live
> under drivers, there is a clash of naming that might create confusion.
> 
> Tidy things up by creating a drivers namespace for interfaces used by
> arch code; achieve this by prepending a 'atd_' (arch topology driver)
> prefix to driver interfaces.

No one knows, nor will they ever remember, what "atd_" means :(

Naming is hard, I know, here's my suggestion:

> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> index 4edae9fe8cdd..e25458d7ee9a 100644
> --- a/include/linux/arch_topology.h
> +++ b/include/linux/arch_topology.h
> @@ -4,14 +4,14 @@
>  #ifndef _LINUX_ARCH_TOPOLOGY_H_
>  #define _LINUX_ARCH_TOPOLOGY_H_
>  
> -void normalize_cpu_capacity(void);
> +void atd_normalize_cpu_capacity(void);

arch_cpu_normalize_capacity();
or
cpu_normalize_capacity();

Why do you care if this is "arch" or not, of course it's arch-specific
in a way, right?

>  
>  struct device_node;
> -int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> +int atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu);

cpu_parse_capacity();

>  struct sched_domain;
> -unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> +unsigned long atd_scale_cpu_capacity(struct sched_domain *sd, int cpu);

cpu_scale_capacity();

> -void set_capacity_scale(unsigned int cpu, unsigned long capacity);
> +void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity);

wait, where did the cpu go?  This doesn't make much sense, these are all
"capacity" issues, right?  If so, then these should be:
	capacity_normalize_cpu()
	capacity_parse_cpu()
	capacity_scale_cpu()
	capacity_set_scale()

But this is all really topology stuff, right?  Why use "capacity" at
all:
	topology_normalize_cpu()
	topology_parse_cpu()
	topology_scale_cpu()
	topology_set_scale()
?

It's always best to put the "subsystem" name first, we have a bad
history of getting this wrong in the past by putting the verb first, not
the noun.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
@ 2017-05-25 13:18     ` Greg KH
  0 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-25 13:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> Now that some functions that deal with arch topology information live
> under drivers, there is a clash of naming that might create confusion.
> 
> Tidy things up by creating a drivers namespace for interfaces used by
> arch code; achieve this by prepending a 'atd_' (arch topology driver)
> prefix to driver interfaces.

No one knows, nor will they ever remember, what "atd_" means :(

Naming is hard, I know, here's my suggestion:

> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> index 4edae9fe8cdd..e25458d7ee9a 100644
> --- a/include/linux/arch_topology.h
> +++ b/include/linux/arch_topology.h
> @@ -4,14 +4,14 @@
>  #ifndef _LINUX_ARCH_TOPOLOGY_H_
>  #define _LINUX_ARCH_TOPOLOGY_H_
>  
> -void normalize_cpu_capacity(void);
> +void atd_normalize_cpu_capacity(void);

arch_cpu_normalize_capacity();
or
cpu_normalize_capacity();

Why do you care if this is "arch" or not, of course it's arch-specific
in a way, right?

>  
>  struct device_node;
> -int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> +int atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu);

cpu_parse_capacity();

>  struct sched_domain;
> -unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> +unsigned long atd_scale_cpu_capacity(struct sched_domain *sd, int cpu);

cpu_scale_capacity();

> -void set_capacity_scale(unsigned int cpu, unsigned long capacity);
> +void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity);

wait, where did the cpu go?  This doesn't make much sense, these are all
"capacity" issues, right?  If so, then these should be:
	capacity_normalize_cpu()
	capacity_parse_cpu()
	capacity_scale_cpu()
	capacity_set_scale()

But this is all really topology stuff, right?  Why use "capacity" at
all:
	topology_normalize_cpu()
	topology_parse_cpu()
	topology_scale_cpu()
	topology_set_scale()
?

It's always best to put the "subsystem" name first, we have a bad
history of getting this wrong in the past by putting the verb first, not
the noun.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
  2017-05-24 14:45         ` Juri Lelli
  (?)
@ 2017-05-25 13:18           ` Greg KH
  -1 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-25 13:18 UTC (permalink / raw)
  To: Juri Lelli
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	broonie, linux-kernel

On Wed, May 24, 2017 at 03:45:15PM +0100, Juri Lelli wrote:
> Hi,
> 
> On 11/05/17 11:27, Juri Lelli wrote:
> > On 11/05/17 10:59, Greg KH wrote:
> > > On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> > > > Hi Greg,
> > > > 
> > > > I'm not sure what your plans are with this set. Are we targeting 4.12?
> > > > 
> > > > IMHO, if possible, it would be good to have this refactoring in now, so
> > > > that we can base further development on it for the next cycle.
> > > 
> > > It's way too late for new features that have not been in linux-next for
> > > a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
> > > patchset after -rc1 is out, and I get a chance to catch up on pending
> > > patches.
> > > 
> > 
> > Thanks for your reply, I perfecly understand.
> > 
> > OK, let's target 4.13 then. Looking forward to receive your comments
> > after -rc1 is out.
> > 
> 
> Did you have any chance to have a look at this set?

Now reviewed.  Looks fine except for the final names in the last patch,
naming is hard, I know :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-25 13:18           ` Greg KH
  0 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-25 13:18 UTC (permalink / raw)
  To: Juri Lelli
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-lFZ/pmaqli7XmaaqVzeoHQ, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, morten.rasmussen-5wv7dgnIgG8,
	dietmar.eggemann-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Wed, May 24, 2017 at 03:45:15PM +0100, Juri Lelli wrote:
> Hi,
> 
> On 11/05/17 11:27, Juri Lelli wrote:
> > On 11/05/17 10:59, Greg KH wrote:
> > > On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> > > > Hi Greg,
> > > > 
> > > > I'm not sure what your plans are with this set. Are we targeting 4.12?
> > > > 
> > > > IMHO, if possible, it would be good to have this refactoring in now, so
> > > > that we can base further development on it for the next cycle.
> > > 
> > > It's way too late for new features that have not been in linux-next for
> > > a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
> > > patchset after -rc1 is out, and I get a chance to catch up on pending
> > > patches.
> > > 
> > 
> > Thanks for your reply, I perfecly understand.
> > 
> > OK, let's target 4.13 then. Looking forward to receive your comments
> > after -rc1 is out.
> > 
> 
> Did you have any chance to have a look at this set?

Now reviewed.  Looks fine except for the final names in the last patch,
naming is hard, I know :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-25 13:18           ` Greg KH
  0 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-25 13:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, May 24, 2017 at 03:45:15PM +0100, Juri Lelli wrote:
> Hi,
> 
> On 11/05/17 11:27, Juri Lelli wrote:
> > On 11/05/17 10:59, Greg KH wrote:
> > > On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> > > > Hi Greg,
> > > > 
> > > > I'm not sure what your plans are with this set. Are we targeting 4.12?
> > > > 
> > > > IMHO, if possible, it would be good to have this refactoring in now, so
> > > > that we can base further development on it for the next cycle.
> > > 
> > > It's way too late for new features that have not been in linux-next for
> > > a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
> > > patchset after -rc1 is out, and I get a chance to catch up on pending
> > > patches.
> > > 
> > 
> > Thanks for your reply, I perfecly understand.
> > 
> > OK, let's target 4.13 then. Looking forward to receive your comments
> > after -rc1 is out.
> > 
> 
> Did you have any chance to have a look at this set?

Now reviewed.  Looks fine except for the final names in the last patch,
naming is hard, I know :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-25 13:30             ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-25 13:30 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-pm, linux-arm-kernel, devicetree, peterz, vincent.guittot,
	robh+dt, mark.rutland, linux, sudeep.holla, lorenzo.pieralisi,
	catalin.marinas, will.deacon, morten.rasmussen, dietmar.eggemann,
	broonie, linux-kernel

On 25/05/17 15:18, Greg KH wrote:
> On Wed, May 24, 2017 at 03:45:15PM +0100, Juri Lelli wrote:
> > Hi,
> > 
> > On 11/05/17 11:27, Juri Lelli wrote:
> > > On 11/05/17 10:59, Greg KH wrote:
> > > > On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> > > > > Hi Greg,
> > > > > 
> > > > > I'm not sure what your plans are with this set. Are we targeting 4.12?
> > > > > 
> > > > > IMHO, if possible, it would be good to have this refactoring in now, so
> > > > > that we can base further development on it for the next cycle.
> > > > 
> > > > It's way too late for new features that have not been in linux-next for
> > > > a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
> > > > patchset after -rc1 is out, and I get a chance to catch up on pending
> > > > patches.
> > > > 
> > > 
> > > Thanks for your reply, I perfecly understand.
> > > 
> > > OK, let's target 4.13 then. Looking forward to receive your comments
> > > after -rc1 is out.
> > > 
> > 
> > Did you have any chance to have a look at this set?
> 
> Now reviewed.  Looks fine except for the final names in the last patch,
> naming is hard, I know :)
> 

Thanks!

Just skimmed through your comments on 8/8. I'll address them in the next
few days and post again.

Best,

- Juri

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-25 13:30             ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-25 13:30 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-lFZ/pmaqli7XmaaqVzeoHQ, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, morten.rasmussen-5wv7dgnIgG8,
	dietmar.eggemann-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 25/05/17 15:18, Greg KH wrote:
> On Wed, May 24, 2017 at 03:45:15PM +0100, Juri Lelli wrote:
> > Hi,
> > 
> > On 11/05/17 11:27, Juri Lelli wrote:
> > > On 11/05/17 10:59, Greg KH wrote:
> > > > On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> > > > > Hi Greg,
> > > > > 
> > > > > I'm not sure what your plans are with this set. Are we targeting 4.12?
> > > > > 
> > > > > IMHO, if possible, it would be good to have this refactoring in now, so
> > > > > that we can base further development on it for the next cycle.
> > > > 
> > > > It's way too late for new features that have not been in linux-next for
> > > > a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
> > > > patchset after -rc1 is out, and I get a chance to catch up on pending
> > > > patches.
> > > > 
> > > 
> > > Thanks for your reply, I perfecly understand.
> > > 
> > > OK, let's target 4.13 then. Looking forward to receive your comments
> > > after -rc1 is out.
> > > 
> > 
> > Did you have any chance to have a look at this set?
> 
> Now reviewed.  Looks fine except for the final names in the last patch,
> naming is hard, I know :)
> 

Thanks!

Just skimmed through your comments on 8/8. I'll address them in the next
few days and post again.

Best,

- Juri
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code
@ 2017-05-25 13:30             ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-25 13:30 UTC (permalink / raw)
  To: linux-arm-kernel

On 25/05/17 15:18, Greg KH wrote:
> On Wed, May 24, 2017 at 03:45:15PM +0100, Juri Lelli wrote:
> > Hi,
> > 
> > On 11/05/17 11:27, Juri Lelli wrote:
> > > On 11/05/17 10:59, Greg KH wrote:
> > > > On Thu, May 11, 2017 at 09:48:30AM +0100, Juri Lelli wrote:
> > > > > Hi Greg,
> > > > > 
> > > > > I'm not sure what your plans are with this set. Are we targeting 4.12?
> > > > > 
> > > > > IMHO, if possible, it would be good to have this refactoring in now, so
> > > > > that we can base further development on it for the next cycle.
> > > > 
> > > > It's way too late for new features that have not been in linux-next for
> > > > a few weeks, to get merged into 4.12-rc1, sorry.  I'll review this
> > > > patchset after -rc1 is out, and I get a chance to catch up on pending
> > > > patches.
> > > > 
> > > 
> > > Thanks for your reply, I perfecly understand.
> > > 
> > > OK, let's target 4.13 then. Looking forward to receive your comments
> > > after -rc1 is out.
> > > 
> > 
> > Did you have any chance to have a look at this set?
> 
> Now reviewed.  Looks fine except for the final names in the last patch,
> naming is hard, I know :)
> 

Thanks!

Just skimmed through your comments on 8/8. I'll address them in the next
few days and post again.

Best,

- Juri

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
@ 2017-05-26 10:10       ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-26 10:10 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-kernel, linux-pm, linux-arm-kernel, devicetree, peterz,
	vincent.guittot, robh+dt, mark.rutland, linux, sudeep.holla,
	lorenzo.pieralisi, catalin.marinas, will.deacon,
	morten.rasmussen, dietmar.eggemann, broonie

Hi,

On 25/05/17 15:18, Greg KH wrote:
> On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> > Now that some functions that deal with arch topology information live
> > under drivers, there is a clash of naming that might create confusion.
> > 
> > Tidy things up by creating a drivers namespace for interfaces used by
> > arch code; achieve this by prepending a 'atd_' (arch topology driver)
> > prefix to driver interfaces.
> 
> No one knows, nor will they ever remember, what "atd_" means :(
> 
> Naming is hard, I know, here's my suggestion:
> 
> > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> > index 4edae9fe8cdd..e25458d7ee9a 100644
> > --- a/include/linux/arch_topology.h
> > +++ b/include/linux/arch_topology.h
> > @@ -4,14 +4,14 @@
> >  #ifndef _LINUX_ARCH_TOPOLOGY_H_
> >  #define _LINUX_ARCH_TOPOLOGY_H_
> >  
> > -void normalize_cpu_capacity(void);
> > +void atd_normalize_cpu_capacity(void);
> 
> arch_cpu_normalize_capacity();
> or
> cpu_normalize_capacity();
> 
> Why do you care if this is "arch" or not, of course it's arch-specific
> in a way, right?
> 
> >  
> >  struct device_node;
> > -int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> > +int atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> 
> cpu_parse_capacity();
> 
> >  struct sched_domain;
> > -unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> > +unsigned long atd_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> 
> cpu_scale_capacity();
> 
> > -void set_capacity_scale(unsigned int cpu, unsigned long capacity);
> > +void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity);
> 
> wait, where did the cpu go?  This doesn't make much sense, these are all
> "capacity" issues, right?  If so, then these should be:
> 	capacity_normalize_cpu()
> 	capacity_parse_cpu()
> 	capacity_scale_cpu()
> 	capacity_set_scale()
> 
> But this is all really topology stuff, right?  Why use "capacity" at
> all:
> 	topology_normalize_cpu()
> 	topology_parse_cpu()
> 	topology_scale_cpu()
> 	topology_set_scale()
> ?
> 
> It's always best to put the "subsystem" name first, we have a bad
> history of getting this wrong in the past by putting the verb first, not
> the noun.
> 

topology_ works for me. However, I'd keep "capacity" in the names, as we
might need to topology_normalize_cpu_somethingelse() (etc.) in the
future?

Updated patch follows. I kept Catalin and Russell's acks as I only
renamed the functions, please shout if that's not OK.

Greg, if you are fine with this approach, do you still want a complete
v5 of the set or can you pick this up?

Thanks,

- Juri

--->8---
>From 56cc1d184bff0e14809ec83043e7a2179a05ccdf Mon Sep 17 00:00:00 2001
From: Juri Lelli <juri.lelli@arm.com>
Date: Wed, 1 Feb 2017 18:46:54 +0000
Subject: [PATCH 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology
 interfaces

Now that some functions that deal with arch topology information live
under drivers, there is a clash of naming that might create confusion.

Tidy things up by creating a topology namespace for interfaces used by
arch code; achieve this by prepending a 'topology_' prefix to driver
interfaces.

Signed-off-by: Juri Lelli <juri.lelli@arm.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/kernel/topology.c    |  8 ++++----
 arch/arm64/kernel/topology.c  |  4 ++--
 drivers/base/arch_topology.c  | 20 ++++++++++----------
 include/linux/arch_topology.h |  8 ++++----
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 557be4f1d2d7..dad8ca071133 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -111,7 +111,7 @@ static void __init parse_dt_topology(void)
 			continue;
 		}
 
-		if (parse_cpu_capacity(cn, cpu)) {
+		if (topology_parse_cpu_capacity(cn, cpu)) {
 			of_node_put(cn);
 			continue;
 		}
@@ -160,7 +160,7 @@ static void __init parse_dt_topology(void)
 				>> (SCHED_CAPACITY_SHIFT-1)) + 1;
 
 	if (cap_from_dt)
-		normalize_cpu_capacity();
+		topology_normalize_cpu_capacity();
 }
 
 /*
@@ -173,10 +173,10 @@ static void update_cpu_capacity(unsigned int cpu)
 	if (!cpu_capacity(cpu) || cap_from_dt)
 		return;
 
-	set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+	topology_set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
 
 	pr_info("CPU%u: update cpu_capacity %lu\n",
-		cpu, arch_scale_cpu_capacity(NULL, cpu));
+		cpu, topology_scale_cpu_capacity(NULL, cpu));
 }
 
 #else
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 255230c3e835..7290ee26e535 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -39,7 +39,7 @@ static int __init get_cpu_for_node(struct device_node *node)
 
 	for_each_possible_cpu(cpu) {
 		if (of_get_cpu_node(cpu, NULL) == cpu_node) {
-			parse_cpu_capacity(cpu_node, cpu);
+			topology_parse_cpu_capacity(cpu_node, cpu);
 			of_node_put(cpu_node);
 			return cpu;
 		}
@@ -191,7 +191,7 @@ static int __init parse_dt_topology(void)
 	if (ret != 0)
 		goto out_map;
 
-	normalize_cpu_capacity();
+	topology_normalize_cpu_capacity();
 
 	/*
 	 * Check that all cores are in the topology; the SMP code will
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 76c19aa0d82f..2012de4f2ed7 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -25,12 +25,12 @@
 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)
+unsigned long topology_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)
+void topology_set_capacity_scale(unsigned int cpu, unsigned long capacity)
 {
 	per_cpu(cpu_scale, cpu) = capacity;
 }
@@ -42,7 +42,7 @@ static ssize_t cpu_capacity_show(struct device *dev,
 	struct cpu *cpu = container_of(dev, struct cpu, dev);
 
 	return sprintf(buf, "%lu\n",
-			arch_scale_cpu_capacity(NULL, cpu->dev.id));
+			topology_scale_cpu_capacity(NULL, cpu->dev.id));
 }
 
 static ssize_t cpu_capacity_store(struct device *dev,
@@ -67,7 +67,7 @@ static ssize_t cpu_capacity_store(struct device *dev,
 
 	mutex_lock(&cpu_scale_mutex);
 	for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
-		set_capacity_scale(i, new_capacity);
+		topology_set_capacity_scale(i, new_capacity);
 	mutex_unlock(&cpu_scale_mutex);
 
 	return count;
@@ -98,7 +98,7 @@ static u32 capacity_scale;
 static u32 *raw_capacity;
 static bool cap_parsing_failed;
 
-void normalize_cpu_capacity(void)
+void topology_normalize_cpu_capacity(void)
 {
 	u64 capacity;
 	int cpu;
@@ -113,14 +113,14 @@ void normalize_cpu_capacity(void)
 			 cpu, raw_capacity[cpu]);
 		capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
 			/ capacity_scale;
-		set_capacity_scale(cpu, capacity);
+		topology_set_capacity_scale(cpu, capacity);
 		pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
-			cpu, arch_scale_cpu_capacity(NULL, cpu));
+			cpu, topology_scale_cpu_capacity(NULL, cpu));
 	}
 	mutex_unlock(&cpu_scale_mutex);
 }
 
-int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
+int __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
 {
 	int ret = 1;
 	u32 cpu_capacity;
@@ -185,12 +185,12 @@ init_cpu_capacity_callback(struct notifier_block *nb,
 			       cpus_to_visit,
 			       policy->related_cpus);
 		for_each_cpu(cpu, policy->related_cpus) {
-			raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
+			raw_capacity[cpu] = topology_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();
+			topology_normalize_cpu_capacity();
 			kfree(raw_capacity);
 			pr_debug("cpu_capacity: parsing done\n");
 			cap_parsing_done = true;
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index 4edae9fe8cdd..553afafe0f8a 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -4,14 +4,14 @@
 #ifndef _LINUX_ARCH_TOPOLOGY_H_
 #define _LINUX_ARCH_TOPOLOGY_H_
 
-void normalize_cpu_capacity(void);
+void topology_normalize_cpu_capacity(void);
 
 struct device_node;
-int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
+int topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
 
 struct sched_domain;
-unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
+unsigned long topology_scale_cpu_capacity(struct sched_domain *sd, int cpu);
 
-void set_capacity_scale(unsigned int cpu, unsigned long capacity);
+void topology_set_capacity_scale(unsigned int cpu, unsigned long capacity);
 
 #endif /* _LINUX_ARCH_TOPOLOGY_H_ */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
@ 2017-05-26 10:10       ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-26 10:10 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-lFZ/pmaqli7XmaaqVzeoHQ, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, morten.rasmussen-5wv7dgnIgG8,
	dietmar.eggemann-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A

Hi,

On 25/05/17 15:18, Greg KH wrote:
> On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> > Now that some functions that deal with arch topology information live
> > under drivers, there is a clash of naming that might create confusion.
> > 
> > Tidy things up by creating a drivers namespace for interfaces used by
> > arch code; achieve this by prepending a 'atd_' (arch topology driver)
> > prefix to driver interfaces.
> 
> No one knows, nor will they ever remember, what "atd_" means :(
> 
> Naming is hard, I know, here's my suggestion:
> 
> > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> > index 4edae9fe8cdd..e25458d7ee9a 100644
> > --- a/include/linux/arch_topology.h
> > +++ b/include/linux/arch_topology.h
> > @@ -4,14 +4,14 @@
> >  #ifndef _LINUX_ARCH_TOPOLOGY_H_
> >  #define _LINUX_ARCH_TOPOLOGY_H_
> >  
> > -void normalize_cpu_capacity(void);
> > +void atd_normalize_cpu_capacity(void);
> 
> arch_cpu_normalize_capacity();
> or
> cpu_normalize_capacity();
> 
> Why do you care if this is "arch" or not, of course it's arch-specific
> in a way, right?
> 
> >  
> >  struct device_node;
> > -int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> > +int atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> 
> cpu_parse_capacity();
> 
> >  struct sched_domain;
> > -unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> > +unsigned long atd_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> 
> cpu_scale_capacity();
> 
> > -void set_capacity_scale(unsigned int cpu, unsigned long capacity);
> > +void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity);
> 
> wait, where did the cpu go?  This doesn't make much sense, these are all
> "capacity" issues, right?  If so, then these should be:
> 	capacity_normalize_cpu()
> 	capacity_parse_cpu()
> 	capacity_scale_cpu()
> 	capacity_set_scale()
> 
> But this is all really topology stuff, right?  Why use "capacity" at
> all:
> 	topology_normalize_cpu()
> 	topology_parse_cpu()
> 	topology_scale_cpu()
> 	topology_set_scale()
> ?
> 
> It's always best to put the "subsystem" name first, we have a bad
> history of getting this wrong in the past by putting the verb first, not
> the noun.
> 

topology_ works for me. However, I'd keep "capacity" in the names, as we
might need to topology_normalize_cpu_somethingelse() (etc.) in the
future?

Updated patch follows. I kept Catalin and Russell's acks as I only
renamed the functions, please shout if that's not OK.

Greg, if you are fine with this approach, do you still want a complete
v5 of the set or can you pick this up?

Thanks,

- Juri

--->8---
>From 56cc1d184bff0e14809ec83043e7a2179a05ccdf Mon Sep 17 00:00:00 2001
From: Juri Lelli <juri.lelli-5wv7dgnIgG8@public.gmane.org>
Date: Wed, 1 Feb 2017 18:46:54 +0000
Subject: [PATCH 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology
 interfaces

Now that some functions that deal with arch topology information live
under drivers, there is a clash of naming that might create confusion.

Tidy things up by creating a topology namespace for interfaces used by
arch code; achieve this by prepending a 'topology_' prefix to driver
interfaces.

Signed-off-by: Juri Lelli <juri.lelli-5wv7dgnIgG8@public.gmane.org>
Acked-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
Acked-by: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm/kernel/topology.c    |  8 ++++----
 arch/arm64/kernel/topology.c  |  4 ++--
 drivers/base/arch_topology.c  | 20 ++++++++++----------
 include/linux/arch_topology.h |  8 ++++----
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 557be4f1d2d7..dad8ca071133 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -111,7 +111,7 @@ static void __init parse_dt_topology(void)
 			continue;
 		}
 
-		if (parse_cpu_capacity(cn, cpu)) {
+		if (topology_parse_cpu_capacity(cn, cpu)) {
 			of_node_put(cn);
 			continue;
 		}
@@ -160,7 +160,7 @@ static void __init parse_dt_topology(void)
 				>> (SCHED_CAPACITY_SHIFT-1)) + 1;
 
 	if (cap_from_dt)
-		normalize_cpu_capacity();
+		topology_normalize_cpu_capacity();
 }
 
 /*
@@ -173,10 +173,10 @@ static void update_cpu_capacity(unsigned int cpu)
 	if (!cpu_capacity(cpu) || cap_from_dt)
 		return;
 
-	set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+	topology_set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
 
 	pr_info("CPU%u: update cpu_capacity %lu\n",
-		cpu, arch_scale_cpu_capacity(NULL, cpu));
+		cpu, topology_scale_cpu_capacity(NULL, cpu));
 }
 
 #else
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 255230c3e835..7290ee26e535 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -39,7 +39,7 @@ static int __init get_cpu_for_node(struct device_node *node)
 
 	for_each_possible_cpu(cpu) {
 		if (of_get_cpu_node(cpu, NULL) == cpu_node) {
-			parse_cpu_capacity(cpu_node, cpu);
+			topology_parse_cpu_capacity(cpu_node, cpu);
 			of_node_put(cpu_node);
 			return cpu;
 		}
@@ -191,7 +191,7 @@ static int __init parse_dt_topology(void)
 	if (ret != 0)
 		goto out_map;
 
-	normalize_cpu_capacity();
+	topology_normalize_cpu_capacity();
 
 	/*
 	 * Check that all cores are in the topology; the SMP code will
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 76c19aa0d82f..2012de4f2ed7 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -25,12 +25,12 @@
 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)
+unsigned long topology_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)
+void topology_set_capacity_scale(unsigned int cpu, unsigned long capacity)
 {
 	per_cpu(cpu_scale, cpu) = capacity;
 }
@@ -42,7 +42,7 @@ static ssize_t cpu_capacity_show(struct device *dev,
 	struct cpu *cpu = container_of(dev, struct cpu, dev);
 
 	return sprintf(buf, "%lu\n",
-			arch_scale_cpu_capacity(NULL, cpu->dev.id));
+			topology_scale_cpu_capacity(NULL, cpu->dev.id));
 }
 
 static ssize_t cpu_capacity_store(struct device *dev,
@@ -67,7 +67,7 @@ static ssize_t cpu_capacity_store(struct device *dev,
 
 	mutex_lock(&cpu_scale_mutex);
 	for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
-		set_capacity_scale(i, new_capacity);
+		topology_set_capacity_scale(i, new_capacity);
 	mutex_unlock(&cpu_scale_mutex);
 
 	return count;
@@ -98,7 +98,7 @@ static u32 capacity_scale;
 static u32 *raw_capacity;
 static bool cap_parsing_failed;
 
-void normalize_cpu_capacity(void)
+void topology_normalize_cpu_capacity(void)
 {
 	u64 capacity;
 	int cpu;
@@ -113,14 +113,14 @@ void normalize_cpu_capacity(void)
 			 cpu, raw_capacity[cpu]);
 		capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
 			/ capacity_scale;
-		set_capacity_scale(cpu, capacity);
+		topology_set_capacity_scale(cpu, capacity);
 		pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
-			cpu, arch_scale_cpu_capacity(NULL, cpu));
+			cpu, topology_scale_cpu_capacity(NULL, cpu));
 	}
 	mutex_unlock(&cpu_scale_mutex);
 }
 
-int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
+int __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
 {
 	int ret = 1;
 	u32 cpu_capacity;
@@ -185,12 +185,12 @@ init_cpu_capacity_callback(struct notifier_block *nb,
 			       cpus_to_visit,
 			       policy->related_cpus);
 		for_each_cpu(cpu, policy->related_cpus) {
-			raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
+			raw_capacity[cpu] = topology_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();
+			topology_normalize_cpu_capacity();
 			kfree(raw_capacity);
 			pr_debug("cpu_capacity: parsing done\n");
 			cap_parsing_done = true;
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index 4edae9fe8cdd..553afafe0f8a 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -4,14 +4,14 @@
 #ifndef _LINUX_ARCH_TOPOLOGY_H_
 #define _LINUX_ARCH_TOPOLOGY_H_
 
-void normalize_cpu_capacity(void);
+void topology_normalize_cpu_capacity(void);
 
 struct device_node;
-int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
+int topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
 
 struct sched_domain;
-unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
+unsigned long topology_scale_cpu_capacity(struct sched_domain *sd, int cpu);
 
-void set_capacity_scale(unsigned int cpu, unsigned long capacity);
+void topology_set_capacity_scale(unsigned int cpu, unsigned long capacity);
 
 #endif /* _LINUX_ARCH_TOPOLOGY_H_ */
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
@ 2017-05-26 10:10       ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-26 10:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 25/05/17 15:18, Greg KH wrote:
> On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> > Now that some functions that deal with arch topology information live
> > under drivers, there is a clash of naming that might create confusion.
> > 
> > Tidy things up by creating a drivers namespace for interfaces used by
> > arch code; achieve this by prepending a 'atd_' (arch topology driver)
> > prefix to driver interfaces.
> 
> No one knows, nor will they ever remember, what "atd_" means :(
> 
> Naming is hard, I know, here's my suggestion:
> 
> > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> > index 4edae9fe8cdd..e25458d7ee9a 100644
> > --- a/include/linux/arch_topology.h
> > +++ b/include/linux/arch_topology.h
> > @@ -4,14 +4,14 @@
> >  #ifndef _LINUX_ARCH_TOPOLOGY_H_
> >  #define _LINUX_ARCH_TOPOLOGY_H_
> >  
> > -void normalize_cpu_capacity(void);
> > +void atd_normalize_cpu_capacity(void);
> 
> arch_cpu_normalize_capacity();
> or
> cpu_normalize_capacity();
> 
> Why do you care if this is "arch" or not, of course it's arch-specific
> in a way, right?
> 
> >  
> >  struct device_node;
> > -int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> > +int atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> 
> cpu_parse_capacity();
> 
> >  struct sched_domain;
> > -unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> > +unsigned long atd_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> 
> cpu_scale_capacity();
> 
> > -void set_capacity_scale(unsigned int cpu, unsigned long capacity);
> > +void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity);
> 
> wait, where did the cpu go?  This doesn't make much sense, these are all
> "capacity" issues, right?  If so, then these should be:
> 	capacity_normalize_cpu()
> 	capacity_parse_cpu()
> 	capacity_scale_cpu()
> 	capacity_set_scale()
> 
> But this is all really topology stuff, right?  Why use "capacity" at
> all:
> 	topology_normalize_cpu()
> 	topology_parse_cpu()
> 	topology_scale_cpu()
> 	topology_set_scale()
> ?
> 
> It's always best to put the "subsystem" name first, we have a bad
> history of getting this wrong in the past by putting the verb first, not
> the noun.
> 

topology_ works for me. However, I'd keep "capacity" in the names, as we
might need to topology_normalize_cpu_somethingelse() (etc.) in the
future?

Updated patch follows. I kept Catalin and Russell's acks as I only
renamed the functions, please shout if that's not OK.

Greg, if you are fine with this approach, do you still want a complete
v5 of the set or can you pick this up?

Thanks,

- Juri

--->8---
>From 56cc1d184bff0e14809ec83043e7a2179a05ccdf Mon Sep 17 00:00:00 2001
From: Juri Lelli <juri.lelli@arm.com>
Date: Wed, 1 Feb 2017 18:46:54 +0000
Subject: [PATCH 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology
 interfaces

Now that some functions that deal with arch topology information live
under drivers, there is a clash of naming that might create confusion.

Tidy things up by creating a topology namespace for interfaces used by
arch code; achieve this by prepending a 'topology_' prefix to driver
interfaces.

Signed-off-by: Juri Lelli <juri.lelli@arm.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/kernel/topology.c    |  8 ++++----
 arch/arm64/kernel/topology.c  |  4 ++--
 drivers/base/arch_topology.c  | 20 ++++++++++----------
 include/linux/arch_topology.h |  8 ++++----
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index 557be4f1d2d7..dad8ca071133 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -111,7 +111,7 @@ static void __init parse_dt_topology(void)
 			continue;
 		}
 
-		if (parse_cpu_capacity(cn, cpu)) {
+		if (topology_parse_cpu_capacity(cn, cpu)) {
 			of_node_put(cn);
 			continue;
 		}
@@ -160,7 +160,7 @@ static void __init parse_dt_topology(void)
 				>> (SCHED_CAPACITY_SHIFT-1)) + 1;
 
 	if (cap_from_dt)
-		normalize_cpu_capacity();
+		topology_normalize_cpu_capacity();
 }
 
 /*
@@ -173,10 +173,10 @@ static void update_cpu_capacity(unsigned int cpu)
 	if (!cpu_capacity(cpu) || cap_from_dt)
 		return;
 
-	set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
+	topology_set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
 
 	pr_info("CPU%u: update cpu_capacity %lu\n",
-		cpu, arch_scale_cpu_capacity(NULL, cpu));
+		cpu, topology_scale_cpu_capacity(NULL, cpu));
 }
 
 #else
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 255230c3e835..7290ee26e535 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -39,7 +39,7 @@ static int __init get_cpu_for_node(struct device_node *node)
 
 	for_each_possible_cpu(cpu) {
 		if (of_get_cpu_node(cpu, NULL) == cpu_node) {
-			parse_cpu_capacity(cpu_node, cpu);
+			topology_parse_cpu_capacity(cpu_node, cpu);
 			of_node_put(cpu_node);
 			return cpu;
 		}
@@ -191,7 +191,7 @@ static int __init parse_dt_topology(void)
 	if (ret != 0)
 		goto out_map;
 
-	normalize_cpu_capacity();
+	topology_normalize_cpu_capacity();
 
 	/*
 	 * Check that all cores are in the topology; the SMP code will
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 76c19aa0d82f..2012de4f2ed7 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -25,12 +25,12 @@
 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)
+unsigned long topology_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)
+void topology_set_capacity_scale(unsigned int cpu, unsigned long capacity)
 {
 	per_cpu(cpu_scale, cpu) = capacity;
 }
@@ -42,7 +42,7 @@ static ssize_t cpu_capacity_show(struct device *dev,
 	struct cpu *cpu = container_of(dev, struct cpu, dev);
 
 	return sprintf(buf, "%lu\n",
-			arch_scale_cpu_capacity(NULL, cpu->dev.id));
+			topology_scale_cpu_capacity(NULL, cpu->dev.id));
 }
 
 static ssize_t cpu_capacity_store(struct device *dev,
@@ -67,7 +67,7 @@ static ssize_t cpu_capacity_store(struct device *dev,
 
 	mutex_lock(&cpu_scale_mutex);
 	for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
-		set_capacity_scale(i, new_capacity);
+		topology_set_capacity_scale(i, new_capacity);
 	mutex_unlock(&cpu_scale_mutex);
 
 	return count;
@@ -98,7 +98,7 @@ static u32 capacity_scale;
 static u32 *raw_capacity;
 static bool cap_parsing_failed;
 
-void normalize_cpu_capacity(void)
+void topology_normalize_cpu_capacity(void)
 {
 	u64 capacity;
 	int cpu;
@@ -113,14 +113,14 @@ void normalize_cpu_capacity(void)
 			 cpu, raw_capacity[cpu]);
 		capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
 			/ capacity_scale;
-		set_capacity_scale(cpu, capacity);
+		topology_set_capacity_scale(cpu, capacity);
 		pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
-			cpu, arch_scale_cpu_capacity(NULL, cpu));
+			cpu, topology_scale_cpu_capacity(NULL, cpu));
 	}
 	mutex_unlock(&cpu_scale_mutex);
 }
 
-int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
+int __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
 {
 	int ret = 1;
 	u32 cpu_capacity;
@@ -185,12 +185,12 @@ init_cpu_capacity_callback(struct notifier_block *nb,
 			       cpus_to_visit,
 			       policy->related_cpus);
 		for_each_cpu(cpu, policy->related_cpus) {
-			raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
+			raw_capacity[cpu] = topology_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();
+			topology_normalize_cpu_capacity();
 			kfree(raw_capacity);
 			pr_debug("cpu_capacity: parsing done\n");
 			cap_parsing_done = true;
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index 4edae9fe8cdd..553afafe0f8a 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -4,14 +4,14 @@
 #ifndef _LINUX_ARCH_TOPOLOGY_H_
 #define _LINUX_ARCH_TOPOLOGY_H_
 
-void normalize_cpu_capacity(void);
+void topology_normalize_cpu_capacity(void);
 
 struct device_node;
-int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
+int topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
 
 struct sched_domain;
-unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
+unsigned long topology_scale_cpu_capacity(struct sched_domain *sd, int cpu);
 
-void set_capacity_scale(unsigned int cpu, unsigned long capacity);
+void topology_set_capacity_scale(unsigned int cpu, unsigned long capacity);
 
 #endif /* _LINUX_ARCH_TOPOLOGY_H_ */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
  2017-05-26 10:10       ` Juri Lelli
  (?)
@ 2017-05-26 18:36         ` Greg KH
  -1 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-26 18:36 UTC (permalink / raw)
  To: Juri Lelli
  Cc: linux-kernel, linux-pm, linux-arm-kernel, devicetree, peterz,
	vincent.guittot, robh+dt, mark.rutland, linux, sudeep.holla,
	lorenzo.pieralisi, catalin.marinas, will.deacon,
	morten.rasmussen, dietmar.eggemann, broonie

On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
> Hi,
> 
> On 25/05/17 15:18, Greg KH wrote:
> > On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> > > Now that some functions that deal with arch topology information live
> > > under drivers, there is a clash of naming that might create confusion.
> > > 
> > > Tidy things up by creating a drivers namespace for interfaces used by
> > > arch code; achieve this by prepending a 'atd_' (arch topology driver)
> > > prefix to driver interfaces.
> > 
> > No one knows, nor will they ever remember, what "atd_" means :(
> > 
> > Naming is hard, I know, here's my suggestion:
> > 
> > > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> > > index 4edae9fe8cdd..e25458d7ee9a 100644
> > > --- a/include/linux/arch_topology.h
> > > +++ b/include/linux/arch_topology.h
> > > @@ -4,14 +4,14 @@
> > >  #ifndef _LINUX_ARCH_TOPOLOGY_H_
> > >  #define _LINUX_ARCH_TOPOLOGY_H_
> > >  
> > > -void normalize_cpu_capacity(void);
> > > +void atd_normalize_cpu_capacity(void);
> > 
> > arch_cpu_normalize_capacity();
> > or
> > cpu_normalize_capacity();
> > 
> > Why do you care if this is "arch" or not, of course it's arch-specific
> > in a way, right?
> > 
> > >  
> > >  struct device_node;
> > > -int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> > > +int atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> > 
> > cpu_parse_capacity();
> > 
> > >  struct sched_domain;
> > > -unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> > > +unsigned long atd_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> > 
> > cpu_scale_capacity();
> > 
> > > -void set_capacity_scale(unsigned int cpu, unsigned long capacity);
> > > +void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity);
> > 
> > wait, where did the cpu go?  This doesn't make much sense, these are all
> > "capacity" issues, right?  If so, then these should be:
> > 	capacity_normalize_cpu()
> > 	capacity_parse_cpu()
> > 	capacity_scale_cpu()
> > 	capacity_set_scale()
> > 
> > But this is all really topology stuff, right?  Why use "capacity" at
> > all:
> > 	topology_normalize_cpu()
> > 	topology_parse_cpu()
> > 	topology_scale_cpu()
> > 	topology_set_scale()
> > ?
> > 
> > It's always best to put the "subsystem" name first, we have a bad
> > history of getting this wrong in the past by putting the verb first, not
> > the noun.
> > 
> 
> topology_ works for me. However, I'd keep "capacity" in the names, as we
> might need to topology_normalize_cpu_somethingelse() (etc.) in the
> future?

Worry about the future, in the future.  Change the names then, _IF_ it
becomes an issue.  Try to be short and simple please.

> Updated patch follows. I kept Catalin and Russell's acks as I only
> renamed the functions, please shout if that's not OK.
> 
> Greg, if you are fine with this approach, do you still want a complete
> v5 of the set or can you pick this up?

Am I the one who is supposed to take all of these arm-specific patches?
If so, that's fine, but I need to have acks from the arm maintainers...

Oh, and drop "capacity" please :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
@ 2017-05-26 18:36         ` Greg KH
  0 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-26 18:36 UTC (permalink / raw)
  To: Juri Lelli
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
	vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-lFZ/pmaqli7XmaaqVzeoHQ, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, morten.rasmussen-5wv7dgnIgG8,
	dietmar.eggemann-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A

On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
> Hi,
> 
> On 25/05/17 15:18, Greg KH wrote:
> > On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> > > Now that some functions that deal with arch topology information live
> > > under drivers, there is a clash of naming that might create confusion.
> > > 
> > > Tidy things up by creating a drivers namespace for interfaces used by
> > > arch code; achieve this by prepending a 'atd_' (arch topology driver)
> > > prefix to driver interfaces.
> > 
> > No one knows, nor will they ever remember, what "atd_" means :(
> > 
> > Naming is hard, I know, here's my suggestion:
> > 
> > > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> > > index 4edae9fe8cdd..e25458d7ee9a 100644
> > > --- a/include/linux/arch_topology.h
> > > +++ b/include/linux/arch_topology.h
> > > @@ -4,14 +4,14 @@
> > >  #ifndef _LINUX_ARCH_TOPOLOGY_H_
> > >  #define _LINUX_ARCH_TOPOLOGY_H_
> > >  
> > > -void normalize_cpu_capacity(void);
> > > +void atd_normalize_cpu_capacity(void);
> > 
> > arch_cpu_normalize_capacity();
> > or
> > cpu_normalize_capacity();
> > 
> > Why do you care if this is "arch" or not, of course it's arch-specific
> > in a way, right?
> > 
> > >  
> > >  struct device_node;
> > > -int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> > > +int atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> > 
> > cpu_parse_capacity();
> > 
> > >  struct sched_domain;
> > > -unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> > > +unsigned long atd_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> > 
> > cpu_scale_capacity();
> > 
> > > -void set_capacity_scale(unsigned int cpu, unsigned long capacity);
> > > +void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity);
> > 
> > wait, where did the cpu go?  This doesn't make much sense, these are all
> > "capacity" issues, right?  If so, then these should be:
> > 	capacity_normalize_cpu()
> > 	capacity_parse_cpu()
> > 	capacity_scale_cpu()
> > 	capacity_set_scale()
> > 
> > But this is all really topology stuff, right?  Why use "capacity" at
> > all:
> > 	topology_normalize_cpu()
> > 	topology_parse_cpu()
> > 	topology_scale_cpu()
> > 	topology_set_scale()
> > ?
> > 
> > It's always best to put the "subsystem" name first, we have a bad
> > history of getting this wrong in the past by putting the verb first, not
> > the noun.
> > 
> 
> topology_ works for me. However, I'd keep "capacity" in the names, as we
> might need to topology_normalize_cpu_somethingelse() (etc.) in the
> future?

Worry about the future, in the future.  Change the names then, _IF_ it
becomes an issue.  Try to be short and simple please.

> Updated patch follows. I kept Catalin and Russell's acks as I only
> renamed the functions, please shout if that's not OK.
> 
> Greg, if you are fine with this approach, do you still want a complete
> v5 of the set or can you pick this up?

Am I the one who is supposed to take all of these arm-specific patches?
If so, that's fine, but I need to have acks from the arm maintainers...

Oh, and drop "capacity" please :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
@ 2017-05-26 18:36         ` Greg KH
  0 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-26 18:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
> Hi,
> 
> On 25/05/17 15:18, Greg KH wrote:
> > On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> > > Now that some functions that deal with arch topology information live
> > > under drivers, there is a clash of naming that might create confusion.
> > > 
> > > Tidy things up by creating a drivers namespace for interfaces used by
> > > arch code; achieve this by prepending a 'atd_' (arch topology driver)
> > > prefix to driver interfaces.
> > 
> > No one knows, nor will they ever remember, what "atd_" means :(
> > 
> > Naming is hard, I know, here's my suggestion:
> > 
> > > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> > > index 4edae9fe8cdd..e25458d7ee9a 100644
> > > --- a/include/linux/arch_topology.h
> > > +++ b/include/linux/arch_topology.h
> > > @@ -4,14 +4,14 @@
> > >  #ifndef _LINUX_ARCH_TOPOLOGY_H_
> > >  #define _LINUX_ARCH_TOPOLOGY_H_
> > >  
> > > -void normalize_cpu_capacity(void);
> > > +void atd_normalize_cpu_capacity(void);
> > 
> > arch_cpu_normalize_capacity();
> > or
> > cpu_normalize_capacity();
> > 
> > Why do you care if this is "arch" or not, of course it's arch-specific
> > in a way, right?
> > 
> > >  
> > >  struct device_node;
> > > -int parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> > > +int atd_parse_cpu_capacity(struct device_node *cpu_node, int cpu);
> > 
> > cpu_parse_capacity();
> > 
> > >  struct sched_domain;
> > > -unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> > > +unsigned long atd_scale_cpu_capacity(struct sched_domain *sd, int cpu);
> > 
> > cpu_scale_capacity();
> > 
> > > -void set_capacity_scale(unsigned int cpu, unsigned long capacity);
> > > +void atd_set_capacity_scale(unsigned int cpu, unsigned long capacity);
> > 
> > wait, where did the cpu go?  This doesn't make much sense, these are all
> > "capacity" issues, right?  If so, then these should be:
> > 	capacity_normalize_cpu()
> > 	capacity_parse_cpu()
> > 	capacity_scale_cpu()
> > 	capacity_set_scale()
> > 
> > But this is all really topology stuff, right?  Why use "capacity" at
> > all:
> > 	topology_normalize_cpu()
> > 	topology_parse_cpu()
> > 	topology_scale_cpu()
> > 	topology_set_scale()
> > ?
> > 
> > It's always best to put the "subsystem" name first, we have a bad
> > history of getting this wrong in the past by putting the verb first, not
> > the noun.
> > 
> 
> topology_ works for me. However, I'd keep "capacity" in the names, as we
> might need to topology_normalize_cpu_somethingelse() (etc.) in the
> future?

Worry about the future, in the future.  Change the names then, _IF_ it
becomes an issue.  Try to be short and simple please.

> Updated patch follows. I kept Catalin and Russell's acks as I only
> renamed the functions, please shout if that's not OK.
> 
> Greg, if you are fine with this approach, do you still want a complete
> v5 of the set or can you pick this up?

Am I the one who is supposed to take all of these arm-specific patches?
If so, that's fine, but I need to have acks from the arm maintainers...

Oh, and drop "capacity" please :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
  2017-05-26 18:36         ` Greg KH
@ 2017-05-29  9:20           ` Dietmar Eggemann
  -1 siblings, 0 replies; 63+ messages in thread
From: Dietmar Eggemann @ 2017-05-29  9:20 UTC (permalink / raw)
  To: Greg KH, Juri Lelli
  Cc: linux-kernel, linux-pm, linux-arm-kernel, devicetree, peterz,
	vincent.guittot, robh+dt, mark.rutland, linux, sudeep.holla,
	lorenzo.pieralisi, catalin.marinas, will.deacon,
	morten.rasmussen, broonie

Hi Greg,

On 05/26/2017 08:36 PM, Greg KH wrote:
> On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
>> Hi,
>>
>> On 25/05/17 15:18, Greg KH wrote:
>>> On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:

[...]

>>> But this is all really topology stuff, right?  Why use "capacity" at
>>> all:
>>> 	topology_normalize_cpu()
>>> 	topology_parse_cpu()
>>> 	topology_scale_cpu()
>>> 	topology_set_scale()
>>> ?
>>>
>>> It's always best to put the "subsystem" name first, we have a bad
>>> history of getting this wrong in the past by putting the verb first, not
>>> the noun.
>>>
>>
>> topology_ works for me. However, I'd keep "capacity" in the names, as we
>> might need to topology_normalize_cpu_somethingelse() (etc.) in the
>> future?
> 
> Worry about the future, in the future.  Change the names then, _IF_ it
> becomes an issue.  Try to be short and simple please.
> 
>> Updated patch follows. I kept Catalin and Russell's acks as I only
>> renamed the functions, please shout if that's not OK.
>>
>> Greg, if you are fine with this approach, do you still want a complete
>> v5 of the set or can you pick this up?
> 
> Am I the one who is supposed to take all of these arm-specific patches?
> If so, that's fine, but I need to have acks from the arm maintainers...
> 
> Oh, and drop "capacity" please :)

Once we have driver/base/arch_topology.c in, we want to enable (cpu 
micro-architectural + max frequency (OPPmax)) invariant and frequency 
(OPPmin..OPPmax) invariant load-tracking/accounting in the task 
scheduler for arm and arm64.

The way to do this is to define the task scheduler interfaces 
arch_scale_cpu_capacity() and arch_scale_freq_capacity() in arch 
specific code:

#define arch_scale_cpu_capacity topology_scale_cpu_capacity
#define arch_scale_freq_capacity topology_scale_freq_capacity

In case an arch is not defining them, the default definitions in 
kernel/sched/sched.h are used.

So topology_scale_cpu() wouldn't be correct since we scale the 
_capacity_ by the micro-architectural differences (hence cpu) and not 
the cpu.

Likewise we will have a function topology_scale_freq_capacity indicating 
that we scale the capacity by the frequency.

Or would you prefer something like topology_scale_capacity_by_cpu() and 
topology_scale_capacity_by_freq()?

-- Dietmar

[...]

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
@ 2017-05-29  9:20           ` Dietmar Eggemann
  0 siblings, 0 replies; 63+ messages in thread
From: Dietmar Eggemann @ 2017-05-29  9:20 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Greg,

On 05/26/2017 08:36 PM, Greg KH wrote:
> On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
>> Hi,
>>
>> On 25/05/17 15:18, Greg KH wrote:
>>> On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:

[...]

>>> But this is all really topology stuff, right?  Why use "capacity" at
>>> all:
>>> 	topology_normalize_cpu()
>>> 	topology_parse_cpu()
>>> 	topology_scale_cpu()
>>> 	topology_set_scale()
>>> ?
>>>
>>> It's always best to put the "subsystem" name first, we have a bad
>>> history of getting this wrong in the past by putting the verb first, not
>>> the noun.
>>>
>>
>> topology_ works for me. However, I'd keep "capacity" in the names, as we
>> might need to topology_normalize_cpu_somethingelse() (etc.) in the
>> future?
> 
> Worry about the future, in the future.  Change the names then, _IF_ it
> becomes an issue.  Try to be short and simple please.
> 
>> Updated patch follows. I kept Catalin and Russell's acks as I only
>> renamed the functions, please shout if that's not OK.
>>
>> Greg, if you are fine with this approach, do you still want a complete
>> v5 of the set or can you pick this up?
> 
> Am I the one who is supposed to take all of these arm-specific patches?
> If so, that's fine, but I need to have acks from the arm maintainers...
> 
> Oh, and drop "capacity" please :)

Once we have driver/base/arch_topology.c in, we want to enable (cpu 
micro-architectural + max frequency (OPPmax)) invariant and frequency 
(OPPmin..OPPmax) invariant load-tracking/accounting in the task 
scheduler for arm and arm64.

The way to do this is to define the task scheduler interfaces 
arch_scale_cpu_capacity() and arch_scale_freq_capacity() in arch 
specific code:

#define arch_scale_cpu_capacity topology_scale_cpu_capacity
#define arch_scale_freq_capacity topology_scale_freq_capacity

In case an arch is not defining them, the default definitions in 
kernel/sched/sched.h are used.

So topology_scale_cpu() wouldn't be correct since we scale the 
_capacity_ by the micro-architectural differences (hence cpu) and not 
the cpu.

Likewise we will have a function topology_scale_freq_capacity indicating 
that we scale the capacity by the frequency.

Or would you prefer something like topology_scale_capacity_by_cpu() and 
topology_scale_capacity_by_freq()?

-- Dietmar

[...]

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
  2017-05-29  9:20           ` Dietmar Eggemann
@ 2017-05-29  9:58             ` Greg KH
  -1 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-29  9:58 UTC (permalink / raw)
  To: Dietmar Eggemann
  Cc: Juri Lelli, linux-kernel, linux-pm, linux-arm-kernel, devicetree,
	peterz, vincent.guittot, robh+dt, mark.rutland, linux,
	sudeep.holla, lorenzo.pieralisi, catalin.marinas, will.deacon,
	morten.rasmussen, broonie

On Mon, May 29, 2017 at 11:20:24AM +0200, Dietmar Eggemann wrote:
> Hi Greg,
> 
> On 05/26/2017 08:36 PM, Greg KH wrote:
> > On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
> > > Hi,
> > > 
> > > On 25/05/17 15:18, Greg KH wrote:
> > > > On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> 
> [...]
> 
> > > > But this is all really topology stuff, right?  Why use "capacity" at
> > > > all:
> > > > 	topology_normalize_cpu()
> > > > 	topology_parse_cpu()
> > > > 	topology_scale_cpu()
> > > > 	topology_set_scale()
> > > > ?
> > > > 
> > > > It's always best to put the "subsystem" name first, we have a bad
> > > > history of getting this wrong in the past by putting the verb first, not
> > > > the noun.
> > > > 
> > > 
> > > topology_ works for me. However, I'd keep "capacity" in the names, as we
> > > might need to topology_normalize_cpu_somethingelse() (etc.) in the
> > > future?
> > 
> > Worry about the future, in the future.  Change the names then, _IF_ it
> > becomes an issue.  Try to be short and simple please.
> > 
> > > Updated patch follows. I kept Catalin and Russell's acks as I only
> > > renamed the functions, please shout if that's not OK.
> > > 
> > > Greg, if you are fine with this approach, do you still want a complete
> > > v5 of the set or can you pick this up?
> > 
> > Am I the one who is supposed to take all of these arm-specific patches?
> > If so, that's fine, but I need to have acks from the arm maintainers...
> > 
> > Oh, and drop "capacity" please :)
> 
> Once we have driver/base/arch_topology.c in, we want to enable (cpu
> micro-architectural + max frequency (OPPmax)) invariant and frequency
> (OPPmin..OPPmax) invariant load-tracking/accounting in the task scheduler
> for arm and arm64.
> 
> The way to do this is to define the task scheduler interfaces
> arch_scale_cpu_capacity() and arch_scale_freq_capacity() in arch specific
> code:
> 
> #define arch_scale_cpu_capacity topology_scale_cpu_capacity
> #define arch_scale_freq_capacity topology_scale_freq_capacity
> 
> In case an arch is not defining them, the default definitions in
> kernel/sched/sched.h are used.
> 
> So topology_scale_cpu() wouldn't be correct since we scale the _capacity_ by
> the micro-architectural differences (hence cpu) and not the cpu.
> 
> Likewise we will have a function topology_scale_freq_capacity indicating
> that we scale the capacity by the frequency.
> 
> Or would you prefer something like topology_scale_capacity_by_cpu() and
> topology_scale_capacity_by_freq()?

I think that if you are creating an api that the scheduler will use, you
need to ask the scheduler maintainers/developers what they want to see
here, as that would be up to them, not me...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
@ 2017-05-29  9:58             ` Greg KH
  0 siblings, 0 replies; 63+ messages in thread
From: Greg KH @ 2017-05-29  9:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 29, 2017 at 11:20:24AM +0200, Dietmar Eggemann wrote:
> Hi Greg,
> 
> On 05/26/2017 08:36 PM, Greg KH wrote:
> > On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
> > > Hi,
> > > 
> > > On 25/05/17 15:18, Greg KH wrote:
> > > > On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> 
> [...]
> 
> > > > But this is all really topology stuff, right?  Why use "capacity" at
> > > > all:
> > > > 	topology_normalize_cpu()
> > > > 	topology_parse_cpu()
> > > > 	topology_scale_cpu()
> > > > 	topology_set_scale()
> > > > ?
> > > > 
> > > > It's always best to put the "subsystem" name first, we have a bad
> > > > history of getting this wrong in the past by putting the verb first, not
> > > > the noun.
> > > > 
> > > 
> > > topology_ works for me. However, I'd keep "capacity" in the names, as we
> > > might need to topology_normalize_cpu_somethingelse() (etc.) in the
> > > future?
> > 
> > Worry about the future, in the future.  Change the names then, _IF_ it
> > becomes an issue.  Try to be short and simple please.
> > 
> > > Updated patch follows. I kept Catalin and Russell's acks as I only
> > > renamed the functions, please shout if that's not OK.
> > > 
> > > Greg, if you are fine with this approach, do you still want a complete
> > > v5 of the set or can you pick this up?
> > 
> > Am I the one who is supposed to take all of these arm-specific patches?
> > If so, that's fine, but I need to have acks from the arm maintainers...
> > 
> > Oh, and drop "capacity" please :)
> 
> Once we have driver/base/arch_topology.c in, we want to enable (cpu
> micro-architectural + max frequency (OPPmax)) invariant and frequency
> (OPPmin..OPPmax) invariant load-tracking/accounting in the task scheduler
> for arm and arm64.
> 
> The way to do this is to define the task scheduler interfaces
> arch_scale_cpu_capacity() and arch_scale_freq_capacity() in arch specific
> code:
> 
> #define arch_scale_cpu_capacity topology_scale_cpu_capacity
> #define arch_scale_freq_capacity topology_scale_freq_capacity
> 
> In case an arch is not defining them, the default definitions in
> kernel/sched/sched.h are used.
> 
> So topology_scale_cpu() wouldn't be correct since we scale the _capacity_ by
> the micro-architectural differences (hence cpu) and not the cpu.
> 
> Likewise we will have a function topology_scale_freq_capacity indicating
> that we scale the capacity by the frequency.
> 
> Or would you prefer something like topology_scale_capacity_by_cpu() and
> topology_scale_capacity_by_freq()?

I think that if you are creating an api that the scheduler will use, you
need to ask the scheduler maintainers/developers what they want to see
here, as that would be up to them, not me...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
  2017-05-29  9:58             ` Greg KH
@ 2017-05-29 10:46               ` Dietmar Eggemann
  -1 siblings, 0 replies; 63+ messages in thread
From: Dietmar Eggemann @ 2017-05-29 10:46 UTC (permalink / raw)
  To: Greg KH
  Cc: Juri Lelli, linux-kernel, linux-pm, linux-arm-kernel, devicetree,
	peterz, vincent.guittot, robh+dt, mark.rutland, linux,
	sudeep.holla, lorenzo.pieralisi, catalin.marinas, will.deacon,
	morten.rasmussen, broonie

On 05/29/2017 11:58 AM, Greg KH wrote:
> On Mon, May 29, 2017 at 11:20:24AM +0200, Dietmar Eggemann wrote:
>> Hi Greg,
>>
>> On 05/26/2017 08:36 PM, Greg KH wrote:
>>> On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
>>>> Hi,
>>>>
>>>> On 25/05/17 15:18, Greg KH wrote:
>>>>> On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
>>
>> [...]
>>
>>>>> But this is all really topology stuff, right?  Why use "capacity" at
>>>>> all:
>>>>> 	topology_normalize_cpu()
>>>>> 	topology_parse_cpu()
>>>>> 	topology_scale_cpu()
>>>>> 	topology_set_scale()
>>>>> ?
>>>>>
>>>>> It's always best to put the "subsystem" name first, we have a bad
>>>>> history of getting this wrong in the past by putting the verb first, not
>>>>> the noun.
>>>>>
>>>>
>>>> topology_ works for me. However, I'd keep "capacity" in the names, as we
>>>> might need to topology_normalize_cpu_somethingelse() (etc.) in the
>>>> future?
>>>
>>> Worry about the future, in the future.  Change the names then, _IF_ it
>>> becomes an issue.  Try to be short and simple please.
>>>
>>>> Updated patch follows. I kept Catalin and Russell's acks as I only
>>>> renamed the functions, please shout if that's not OK.
>>>>
>>>> Greg, if you are fine with this approach, do you still want a complete
>>>> v5 of the set or can you pick this up?
>>>
>>> Am I the one who is supposed to take all of these arm-specific patches?
>>> If so, that's fine, but I need to have acks from the arm maintainers...
>>>
>>> Oh, and drop "capacity" please :)
>>
>> Once we have driver/base/arch_topology.c in, we want to enable (cpu
>> micro-architectural + max frequency (OPPmax)) invariant and frequency
>> (OPPmin..OPPmax) invariant load-tracking/accounting in the task scheduler
>> for arm and arm64.
>>
>> The way to do this is to define the task scheduler interfaces
>> arch_scale_cpu_capacity() and arch_scale_freq_capacity() in arch specific
>> code:
>>
>> #define arch_scale_cpu_capacity topology_scale_cpu_capacity
>> #define arch_scale_freq_capacity topology_scale_freq_capacity
>>
>> In case an arch is not defining them, the default definitions in
>> kernel/sched/sched.h are used.
>>
>> So topology_scale_cpu() wouldn't be correct since we scale the _capacity_ by
>> the micro-architectural differences (hence cpu) and not the cpu.
>>
>> Likewise we will have a function topology_scale_freq_capacity indicating
>> that we scale the capacity by the frequency.
>>
>> Or would you prefer something like topology_scale_capacity_by_cpu() and
>> topology_scale_capacity_by_freq()?
> 
> I think that if you are creating an api that the scheduler will use, you
> need to ask the scheduler maintainers/developers what they want to see
> here, as that would be up to them, not me...

The scheduler API exists already. It is arch_scale_cpu_capacity() and 
arch_scale_freq_capacity() in kernel/sched/sched.h. An arch is able to 
overwrite these two functions by defining them (since commit 
8cd5601c5060 and dfbca41f3479):

#define arch_scale_cpu_capacity 'arch implementation of capacity scaling 
by micro-architectural + max frequency (OPPmax)'

#define arch_scale_freq_capacity 'arch implementation of capacity 
scaling by 'frequency ((OPPmin..OPPmax)'

There is no naming convention from the scheduler side on these functions 
though. They should just express what they're doing, scaling capacity by 
something.

Since Juri already uses the former one in this driver, he should name it 
  topology_scale_cpu_capacity() or topology_scale_capacity_by_cpu() or 
something similar.

[...]

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
@ 2017-05-29 10:46               ` Dietmar Eggemann
  0 siblings, 0 replies; 63+ messages in thread
From: Dietmar Eggemann @ 2017-05-29 10:46 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/29/2017 11:58 AM, Greg KH wrote:
> On Mon, May 29, 2017 at 11:20:24AM +0200, Dietmar Eggemann wrote:
>> Hi Greg,
>>
>> On 05/26/2017 08:36 PM, Greg KH wrote:
>>> On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
>>>> Hi,
>>>>
>>>> On 25/05/17 15:18, Greg KH wrote:
>>>>> On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
>>
>> [...]
>>
>>>>> But this is all really topology stuff, right?  Why use "capacity" at
>>>>> all:
>>>>> 	topology_normalize_cpu()
>>>>> 	topology_parse_cpu()
>>>>> 	topology_scale_cpu()
>>>>> 	topology_set_scale()
>>>>> ?
>>>>>
>>>>> It's always best to put the "subsystem" name first, we have a bad
>>>>> history of getting this wrong in the past by putting the verb first, not
>>>>> the noun.
>>>>>
>>>>
>>>> topology_ works for me. However, I'd keep "capacity" in the names, as we
>>>> might need to topology_normalize_cpu_somethingelse() (etc.) in the
>>>> future?
>>>
>>> Worry about the future, in the future.  Change the names then, _IF_ it
>>> becomes an issue.  Try to be short and simple please.
>>>
>>>> Updated patch follows. I kept Catalin and Russell's acks as I only
>>>> renamed the functions, please shout if that's not OK.
>>>>
>>>> Greg, if you are fine with this approach, do you still want a complete
>>>> v5 of the set or can you pick this up?
>>>
>>> Am I the one who is supposed to take all of these arm-specific patches?
>>> If so, that's fine, but I need to have acks from the arm maintainers...
>>>
>>> Oh, and drop "capacity" please :)
>>
>> Once we have driver/base/arch_topology.c in, we want to enable (cpu
>> micro-architectural + max frequency (OPPmax)) invariant and frequency
>> (OPPmin..OPPmax) invariant load-tracking/accounting in the task scheduler
>> for arm and arm64.
>>
>> The way to do this is to define the task scheduler interfaces
>> arch_scale_cpu_capacity() and arch_scale_freq_capacity() in arch specific
>> code:
>>
>> #define arch_scale_cpu_capacity topology_scale_cpu_capacity
>> #define arch_scale_freq_capacity topology_scale_freq_capacity
>>
>> In case an arch is not defining them, the default definitions in
>> kernel/sched/sched.h are used.
>>
>> So topology_scale_cpu() wouldn't be correct since we scale the _capacity_ by
>> the micro-architectural differences (hence cpu) and not the cpu.
>>
>> Likewise we will have a function topology_scale_freq_capacity indicating
>> that we scale the capacity by the frequency.
>>
>> Or would you prefer something like topology_scale_capacity_by_cpu() and
>> topology_scale_capacity_by_freq()?
> 
> I think that if you are creating an api that the scheduler will use, you
> need to ask the scheduler maintainers/developers what they want to see
> here, as that would be up to them, not me...

The scheduler API exists already. It is arch_scale_cpu_capacity() and 
arch_scale_freq_capacity() in kernel/sched/sched.h. An arch is able to 
overwrite these two functions by defining them (since commit 
8cd5601c5060 and dfbca41f3479):

#define arch_scale_cpu_capacity 'arch implementation of capacity scaling 
by micro-architectural + max frequency (OPPmax)'

#define arch_scale_freq_capacity 'arch implementation of capacity 
scaling by 'frequency ((OPPmin..OPPmax)'

There is no naming convention from the scheduler side on these functions 
though. They should just express what they're doing, scaling capacity by 
something.

Since Juri already uses the former one in this driver, he should name it 
  topology_scale_cpu_capacity() or topology_scale_capacity_by_cpu() or 
something similar.

[...]

^ permalink raw reply	[flat|nested] 63+ messages in thread
* Re: [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
  2017-05-29 10:46               ` Dietmar Eggemann
@ 2017-05-30 14:59                 ` Juri Lelli
  -1 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-30 14:59 UTC (permalink / raw)
  To: Greg KH
  Cc: Dietmar Eggemann, linux-kernel, linux-pm, linux-arm-kernel,
	devicetree, peterz, vincent.guittot, robh+dt, mark.rutland,
	linux, sudeep.holla, lorenzo.pieralisi, catalin.marinas,
	will.deacon, morten.rasmussen, broonie

On 29/05/17 12:46, Dietmar Eggemann wrote:
> On 05/29/2017 11:58 AM, Greg KH wrote:
> > On Mon, May 29, 2017 at 11:20:24AM +0200, Dietmar Eggemann wrote:
> > > Hi Greg,
> > > 
> > > On 05/26/2017 08:36 PM, Greg KH wrote:
> > > > On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
> > > > > Hi,
> > > > > 
> > > > > On 25/05/17 15:18, Greg KH wrote:
> > > > > > On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> > > 
> > > [...]
> > > 
> > > > > > But this is all really topology stuff, right?  Why use "capacity" at
> > > > > > all:
> > > > > > 	topology_normalize_cpu()
> > > > > > 	topology_parse_cpu()
> > > > > > 	topology_scale_cpu()
> > > > > > 	topology_set_scale()
> > > > > > ?
> > > > > > 
> > > > > > It's always best to put the "subsystem" name first, we have a bad
> > > > > > history of getting this wrong in the past by putting the verb first, not
> > > > > > the noun.
> > > > > > 
> > > > > 

[...]

> > > > 
> > > > Oh, and drop "capacity" please :)
> > > 

[...]

> > 
> > I think that if you are creating an api that the scheduler will use, you
> > need to ask the scheduler maintainers/developers what they want to see
> > here, as that would be up to them, not me...
> 
> The scheduler API exists already. It is arch_scale_cpu_capacity() and
> arch_scale_freq_capacity() in kernel/sched/sched.h. An arch is able to
> overwrite these two functions by defining them (since commit 8cd5601c5060
> and dfbca41f3479):
> 
> #define arch_scale_cpu_capacity 'arch implementation of capacity scaling by
> micro-architectural + max frequency (OPPmax)'
> 
> #define arch_scale_freq_capacity 'arch implementation of capacity scaling by
> 'frequency ((OPPmin..OPPmax)'
> 
> There is no naming convention from the scheduler side on these functions
> though. They should just express what they're doing, scaling capacity by
> something.
> 

So, discussing this naming with Morten off-line we seemed actually to
agree that the following might adhere even better to what the functions
actually do:

 topology_parse_cpu_capacity() - it parses the raw capacity-dmips-mhz
 values from DT; so it seems OK to leave capacity in the name here

 topology_set_cpu_scale() - it sets the per_cpu cpu_scale variable; so
 this name seems saner that the one with "_capacity"

 topology_get_cpu_scale() - dual of the previous one

 topology_normalize_cpu_scale() - it normalizes cpu_scale variables
 across the system CPUs (and calls topology_set_cpu_scale() to set the
 normalized values)

Greg, does this approack look saner to you as well?
If yes, I'll send a new version of the set shortly.

And, as I already said on IRC, apologies for this naming fight. :)

Thanks,

- Juri

^ permalink raw reply	[flat|nested] 63+ messages in thread
* [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces
@ 2017-05-30 14:59                 ` Juri Lelli
  0 siblings, 0 replies; 63+ messages in thread
From: Juri Lelli @ 2017-05-30 14:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 29/05/17 12:46, Dietmar Eggemann wrote:
> On 05/29/2017 11:58 AM, Greg KH wrote:
> > On Mon, May 29, 2017 at 11:20:24AM +0200, Dietmar Eggemann wrote:
> > > Hi Greg,
> > > 
> > > On 05/26/2017 08:36 PM, Greg KH wrote:
> > > > On Fri, May 26, 2017 at 11:10:32AM +0100, Juri Lelli wrote:
> > > > > Hi,
> > > > > 
> > > > > On 25/05/17 15:18, Greg KH wrote:
> > > > > > On Thu, Apr 20, 2017 at 03:43:16PM +0100, Juri Lelli wrote:
> > > 
> > > [...]
> > > 
> > > > > > But this is all really topology stuff, right?  Why use "capacity" at
> > > > > > all:
> > > > > > 	topology_normalize_cpu()
> > > > > > 	topology_parse_cpu()
> > > > > > 	topology_scale_cpu()
> > > > > > 	topology_set_scale()
> > > > > > ?
> > > > > > 
> > > > > > It's always best to put the "subsystem" name first, we have a bad
> > > > > > history of getting this wrong in the past by putting the verb first, not
> > > > > > the noun.
> > > > > > 
> > > > > 

[...]

> > > > 
> > > > Oh, and drop "capacity" please :)
> > > 

[...]

> > 
> > I think that if you are creating an api that the scheduler will use, you
> > need to ask the scheduler maintainers/developers what they want to see
> > here, as that would be up to them, not me...
> 
> The scheduler API exists already. It is arch_scale_cpu_capacity() and
> arch_scale_freq_capacity() in kernel/sched/sched.h. An arch is able to
> overwrite these two functions by defining them (since commit 8cd5601c5060
> and dfbca41f3479):
> 
> #define arch_scale_cpu_capacity 'arch implementation of capacity scaling by
> micro-architectural + max frequency (OPPmax)'
> 
> #define arch_scale_freq_capacity 'arch implementation of capacity scaling by
> 'frequency ((OPPmin..OPPmax)'
> 
> There is no naming convention from the scheduler side on these functions
> though. They should just express what they're doing, scaling capacity by
> something.
> 

So, discussing this naming with Morten off-line we seemed actually to
agree that the following might adhere even better to what the functions
actually do:

 topology_parse_cpu_capacity() - it parses the raw capacity-dmips-mhz
 values from DT; so it seems OK to leave capacity in the name here

 topology_set_cpu_scale() - it sets the per_cpu cpu_scale variable; so
 this name seems saner that the one with "_capacity"

 topology_get_cpu_scale() - dual of the previous one

 topology_normalize_cpu_scale() - it normalizes cpu_scale variables
 across the system CPUs (and calls topology_set_cpu_scale() to set the
 normalized values)

Greg, does this approack look saner to you as well?
If yes, I'll send a new version of the set shortly.

And, as I already said on IRC, apologies for this naming fight. :)

Thanks,

- Juri

^ permalink raw reply	[flat|nested] 63+ messages in thread
end of thread, other threads:[~2017-05-30 14:59 UTC | newest]

Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-20 14:43 [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code Juri Lelli
2017-04-20 14:43 ` Juri Lelli
2017-04-20 14:43 ` Juri Lelli
2017-04-20 14:43 ` [PATCH v4 1/8] Documentation: arm: fix wrong reference number in DT definition Juri Lelli
2017-04-20 14:43   ` Juri Lelli
2017-04-20 14:43 ` [PATCH v4 2/8] Documentation/ABI: add information about cpu_capacity Juri Lelli
2017-04-20 14:43   ` Juri Lelli
2017-04-20 14:43   ` Juri Lelli
2017-04-20 14:43 ` [PATCH v4 3/8] arm: fix return value of parse_cpu_capacity Juri Lelli
2017-04-20 14:43   ` Juri Lelli
2017-04-20 14:43   ` Juri Lelli
2017-04-20 14:50   ` Vincent Guittot
2017-04-20 14:50     ` Vincent Guittot
2017-04-20 14:43 ` [PATCH v4 4/8] arm: remove wrong CONFIG_PROC_SYSCTL ifdef Juri Lelli
2017-04-20 14:43   ` Juri Lelli
2017-04-20 14:43   ` Juri Lelli
2017-04-20 14:43 ` [PATCH v4 5/8] arm, arm64: factorize common cpu capacity default code Juri Lelli
2017-04-20 14:43   ` Juri Lelli
2017-05-25 13:12   ` Greg KH
2017-05-25 13:12     ` Greg KH
2017-04-20 14:43 ` [PATCH v4 6/8] arm,arm64,drivers: reduce scope of cap_parsing_failed Juri Lelli
2017-04-20 14:43   ` Juri Lelli
2017-04-20 14:43   ` Juri Lelli
2017-05-25 13:13   ` Greg KH
2017-05-25 13:13     ` Greg KH
2017-04-20 14:43 ` [PATCH v4 7/8] arm,arm64,drivers: move externs in a new header file Juri Lelli
2017-04-20 14:43   ` Juri Lelli
2017-05-25 13:13   ` Greg KH
2017-05-25 13:13     ` Greg KH
2017-04-20 14:43 ` [PATCH v4 8/8] arm,arm64,drivers: add a prefix to drivers arch_topology interfaces Juri Lelli
2017-04-20 14:43   ` [PATCH v4 8/8] arm, arm64, drivers: " Juri Lelli
2017-05-25 13:18   ` [PATCH v4 8/8] arm,arm64,drivers: " Greg KH
2017-05-25 13:18     ` Greg KH
2017-05-26 10:10     ` Juri Lelli
2017-05-26 10:10       ` Juri Lelli
2017-05-26 10:10       ` Juri Lelli
2017-05-26 18:36       ` Greg KH
2017-05-26 18:36         ` Greg KH
2017-05-26 18:36         ` Greg KH
2017-05-29  9:20         ` Dietmar Eggemann
2017-05-29  9:20           ` Dietmar Eggemann
2017-05-29  9:58           ` Greg KH
2017-05-29  9:58             ` Greg KH
2017-05-29 10:46             ` Dietmar Eggemann
2017-05-29 10:46               ` Dietmar Eggemann
2017-05-30 14:59               ` Juri Lelli
2017-05-30 14:59                 ` Juri Lelli
2017-05-11  8:48 ` [PATCH v4 0/8] Fix issues and factorize arm/arm64 capacity information code Juri Lelli
2017-05-11  8:48   ` Juri Lelli
2017-05-11  8:48   ` Juri Lelli
2017-05-11  8:59   ` Greg KH
2017-05-11  8:59     ` Greg KH
2017-05-11 10:27     ` Juri Lelli
2017-05-11 10:27       ` Juri Lelli
2017-05-11 10:27       ` Juri Lelli
2017-05-24 14:45       ` Juri Lelli
2017-05-24 14:45         ` Juri Lelli
2017-05-25 13:18         ` Greg KH
2017-05-25 13:18           ` Greg KH
2017-05-25 13:18           ` Greg KH
2017-05-25 13:30           ` Juri Lelli
2017-05-25 13:30             ` Juri Lelli
2017-05-25 13:30             ` Juri Lelli

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.