[PATCH v5 0/8] ARM64: juno: add SCPI mailbox protocol, clock and CPUFreq support

[PATCH v5 0/8] ARM64: juno: add SCPI mailbox protocol, clock and CPUFreq support

[Sudeep Holla]

Hi,

This patch series adds support for:
  1. SCPI(System Control and Power Interface) mailbox protocol driver.
     It uses ARM MHU mailbox controller driver on Juno but can work with
     any mailbox controllers using standard mailbox APIs
  2. Add support for clocks provided by SCP firmware through the SCPI
     interface
  3. Using the existing arm_big_little cpufreq driver and the newly
     added SCPI clock driver, it also adds support for CPU DVFS on
     ARM64 JUNO development platforms.

The SCPI protocol document is available @[1],[2]

Changes v4->v5:
	- Updated the SCPI clock bindings to correct the clock specifier
	  usage and other minor updates as per review feedback
	- Updated clock driver to use SCPI specifier clock specifier
	  decode function
	- Minor reshuffling in Juno DTS files, no functionality change
	- Added ACKs recieved so far

Changes v3->v4:
	- Updated the SCPI binding based on MarkR's feedback
	- Updated SCPI clock driver to incorporate Stephen Boyd's review
	  comments + used clk_set_rate_range to limit the clock range
	- Since no major changes are expected in SCPI DT, updated the
	  Juno DTS to support SCPI and dependent device nodes.

Changes v2->v3:
	- Minor fix in SCPI driver and added Tixy's reviewed-by tag
	- Updated scpi clock driver to incorporate all the comments from
	  Stephen
	- Added Viresh's ack

Changes v1->v2:
	- Updated the token handling in scpi driver as per Tixy's
	  suggestion along with other review comments
	- Removed multiple drivers in scpi clock as Lorenzo suggested
	- Added free_opp_table in scpi-cpufreq as Viresh suggested
	- Separated the DT binding document
	- Moved SCPI protocol driver to drivers/firmware

Regards,
Sudeep

[1] http://community.arm.com/servlet/JiveServlet/download/8401-45-18326/DUI0922B_scp_message_interface.pdf
[2] https://wiki.linaro.org/ARM/Juno?action=AttachFile&do=get&target=DUI0922B_scp_message_interface.pdf
v1: https://lkml.org/lkml/2015/4/27/232
v2: https://lkml.org/lkml/2015/5/14/470
v3: https://lkml.org/lkml/2015/5/27/220
v4: https://lkml.org/lkml/2015/6/8/178


*** BLURB HERE ***

Sudeep Holla (8):
  Documentation: add DT binding for ARM System Control and Power
    Interface(SCPI) protocol
  firmware: add support for ARM System Control and Power Interface(SCPI)
    protocol
  clk: add support for clocks provided by SCP(System Control Processor)
  clk: scpi: add support for cpufreq virtual device
  cpufreq: arm_big_little: add SCPI interface driver
  arm64: dts: add SRAM, MHU mailbox and SCPI support on Juno
  arm64: dts: add CPU topology on Juno
  arm64: dts: add clock support for all the cpus

 Documentation/devicetree/bindings/arm/arm,scpi.txt | 151 +++++
 MAINTAINERS                                        |  10 +
 arch/arm64/boot/dts/arm/juno-base.dtsi             |  54 ++
 arch/arm64/boot/dts/arm/juno-r1.dts                |  32 +
 arch/arm64/boot/dts/arm/juno.dts                   |  32 +
 drivers/clk/Kconfig                                |  10 +
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-scpi.c                             | 324 ++++++++++
 drivers/cpufreq/Kconfig.arm                        |   9 +
 drivers/cpufreq/Makefile                           |   1 +
 drivers/cpufreq/scpi-cpufreq.c                     | 124 ++++
 drivers/firmware/Kconfig                           |  19 +
 drivers/firmware/Makefile                          |   1 +
 drivers/firmware/arm_scpi.c                        | 711 +++++++++++++++++++++
 include/linux/scpi_protocol.h                      |  61 ++
 15 files changed, 1540 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/arm,scpi.txt
 create mode 100644 drivers/clk/clk-scpi.c
 create mode 100644 drivers/cpufreq/scpi-cpufreq.c
 create mode 100644 drivers/firmware/arm_scpi.c
 create mode 100644 include/linux/scpi_protocol.h

-- 
1.9.1

[PATCH v5 1/8] Documentation: add DT binding for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

This patch adds devicetree binding for System Control and Power
Interface (SCPI) Message Protocol used between the Application Cores(AP)
and the System Control Processor(SCP). The MHU peripheral provides a
mechanism for inter-processor communication between SCP's M3 processor
and AP.

SCP offers control and management of the core/cluster power states,
various power domain DVFS including the core/cluster, certain system
clocks configuration, thermal sensors and many others.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
CC: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Jon Medhurst (Tixy) <tixy@linaro.org>
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/arm/arm,scpi.txt | 151 +++++++++++++++++++++
 MAINTAINERS                                        |   6 +
 2 files changed, 157 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/arm,scpi.txt

diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
new file mode 100644
index 000000000000..e21cce646561
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
@@ -0,0 +1,151 @@
+System Control and Power Interface (SCPI) Message Protocol
+----------------------------------------------------------
+
+Firmware implementing the SCPI described in ARM document number ARM DUI 0922B
+("ARM Compute Subsystem SCP: Message Interface Protocols")[0] can be used
+by Linux to initiate various system control and power operations.
+
+Required properties:
+
+- compatible : should be "arm,scpi"
+- mboxes: List of phandle and mailbox channel specifiers
+	  All the channels reserved by remote SCP firmware for use by
+	  SCPI message protocol should be specified in any order
+- shmem : List of phandle pointing to the shared memory(SHM) area between the
+	  processors using these mailboxes for IPC, one for each mailbox
+	  SHM can be any memory reserved for the purpose of this communication
+	  between the processors.
+
+See Documentation/devicetree/bindings/mailbox/mailbox.txt
+for more details about the generic mailbox controller and
+client driver bindings.
+
+Clock bindings for the clocks based on SCPI Message Protocol
+------------------------------------------------------------
+
+This binding uses the common clock binding[1].
+
+Container Node
+==============
+Required properties:
+- compatible : should be "arm,scpi-clocks"
+	       All the clocks provided by SCP firmware via SCPI message
+	       protocol much be listed as sub-nodes under this node.
+
+Sub-nodes
+=========
+Required properties:
+- compatible : shall include one of the following
+	"arm,scpi-dvfs-clocks" - all the clocks that are variable and index based.
+		These clocks don't provide an entire range of values between the
+		limits but only discrete points within the range. The firmware
+		provides the mapping for each such operating frequency and the
+		index associated with it. The firmware also manages the
+		voltage scaling appropriately with the clock scaling.
+	"arm,scpi-variable-clocks" - all the clocks that are variable and provide full
+		range within the specified range. The firmware provides the
+		range of values within a specified range.
+
+Other required properties for all clocks(all from common clock binding):
+- #clock-cells : should be set to 1 as each of the SCPI clocks have multiple
+	outputs.
+- clock-output-names : shall be the corresponding names of the outputs.
+- clock-indices: The identifying number for the clocks(i.e.clock_id) in the
+	node. It can be non linear and hence provide the mapping of identifiers
+	into the clock-output-names array.
+
+SRAM and Shared Memory for SCPI
+-------------------------------
+
+A small area of SRAM is reserved for SCPI communication between application
+processors and SCP.
+
+Required properties:
+- compatible : should be "arm,juno-sram-ns" for Non-secure SRAM on Juno
+
+The rest of the properties should follow the generic mmio-sram description
+found in ../../misc/sysram.txt
+
+Each sub-node represents the reserved area for SCPI.
+
+Required sub-node properties:
+- reg : The base offset and size of the reserved area with the SRAM
+- compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
+	       shared memory on Juno platforms
+
+[0] http://community.arm.com/servlet/JiveServlet/download/8401-45-18326/DUI0922B_scp_message_interface.pdf
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Example:
+
+sram: sram@50000000 {
+	compatible = "arm,juno-sram-ns", "mmio-sram";
+	reg = <0x0 0x50000000 0x0 0x10000>;
+
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges = <0 0x0 0x50000000 0x10000>;
+
+	cpu_scp_lpri: scp-shmem@0 {
+		compatible = "arm,juno-scp-shmem";
+		reg = <0x0 0x200>;
+	};
+
+	cpu_scp_hpri: scp-shmem@200 {
+		compatible = "arm,juno-scp-shmem";
+		reg = <0x200 0x200>;
+	};
+};
+
+mailbox: mailbox0@40000000 {
+	....
+	#mbox-cells = <1>;
+};
+
+scpi_protocol: scpi@2e000000 {
+	compatible = "arm,scpi";
+	mboxes = <&mailbox 0 &mailbox 1>;
+	shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
+
+	clocks {
+		compatible = "arm,scpi-clocks";
+
+		scpi_dvfs: scpi_clocks@0 {
+			compatible = "arm,scpi-dvfs-clocks";
+			#clock-cells = <1>;
+			clock-indices = <0>, <1>, <2>;
+			clock-output-names = "atlclk", "aplclk","gpuclk";
+		};
+		scpi_clk: scpi_clocks@3 {
+			compatible = "arm,scpi-variable-clocks";
+			#clock-cells = <1>;
+			clock-indices = <3>, <4>;
+			clock-output-names = "pxlclk0", "pxlclk1";
+		};
+	};
+};
+
+cpu@0 {
+	...
+	reg = <0 0>;
+	clocks = <&scpi_dvfs 0>;
+};
+
+hdlcd@7ff60000 {
+	...
+	reg = <0 0x7ff60000 0 0x1000>;
+	clocks = <&scpi_clk 4>;
+};
+
+In the above example, the #clock-cells is set to 1 as required.
+scpi_dvfs has 3 output clocks namely: vbig, vlittle and vgpu with 0, 1
+and 2 as clock-indices. scpi_clk has 2 output clocks namely: pxlclk0 and
+pxlclk1 with 3 and 4 as clock-indices.
+
+The first consumer in the example is cpu@0 and it has '0' as the clock
+specifier which points to the first entry in the output clocks of
+scpi_dvfs i.e. "atlclk".
+
+Similarly the second example is hdlcd@7ff60000 and it has pxlclk1 as input
+clock. '4' in the clock specifier here points to the second entry
+in the output clocks of scpi_clocks  i.e. "pxlclk1"
diff --git a/MAINTAINERS b/MAINTAINERS
index a2264167791a..9351b62dbbd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8955,6 +8955,12 @@ W:	http://www.sunplus.com
 S:	Supported
 F:	arch/score/
 
+SYSTEM CONTROL & POWER INTERFACE (SCPI) Message Protocol drivers
+M:	Sudeep Holla <sudeep.holla@arm.com>
+L:	linux-arm-kernel@lists.infradead.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/arm/arm,scpi.txt
+
 SCSI CDROM DRIVER
 M:	Jens Axboe <axboe@kernel.dk>
 L:	linux-scsi@vger.kernel.org
-- 
1.9.1

[PATCH v5 1/8] Documentation: add DT binding for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

This patch adds devicetree binding for System Control and Power
Interface (SCPI) Message Protocol used between the Application Cores(AP)
and the System Control Processor(SCP). The MHU peripheral provides a
mechanism for inter-processor communication between SCP's M3 processor
and AP.

SCP offers control and management of the core/cluster power states,
various power domain DVFS including the core/cluster, certain system
clocks configuration, thermal sensors and many others.

Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
CC: Jassi Brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Liviu Dudau <Liviu.Dudau-5wv7dgnIgG8@public.gmane.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
Cc: Jon Medhurst (Tixy) <tixy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 Documentation/devicetree/bindings/arm/arm,scpi.txt | 151 +++++++++++++++++++++
 MAINTAINERS                                        |   6 +
 2 files changed, 157 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/arm,scpi.txt

diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
new file mode 100644
index 000000000000..e21cce646561
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
@@ -0,0 +1,151 @@
+System Control and Power Interface (SCPI) Message Protocol
+----------------------------------------------------------
+
+Firmware implementing the SCPI described in ARM document number ARM DUI 0922B
+("ARM Compute Subsystem SCP: Message Interface Protocols")[0] can be used
+by Linux to initiate various system control and power operations.
+
+Required properties:
+
+- compatible : should be "arm,scpi"
+- mboxes: List of phandle and mailbox channel specifiers
+	  All the channels reserved by remote SCP firmware for use by
+	  SCPI message protocol should be specified in any order
+- shmem : List of phandle pointing to the shared memory(SHM) area between the
+	  processors using these mailboxes for IPC, one for each mailbox
+	  SHM can be any memory reserved for the purpose of this communication
+	  between the processors.
+
+See Documentation/devicetree/bindings/mailbox/mailbox.txt
+for more details about the generic mailbox controller and
+client driver bindings.
+
+Clock bindings for the clocks based on SCPI Message Protocol
+------------------------------------------------------------
+
+This binding uses the common clock binding[1].
+
+Container Node
+==============
+Required properties:
+- compatible : should be "arm,scpi-clocks"
+	       All the clocks provided by SCP firmware via SCPI message
+	       protocol much be listed as sub-nodes under this node.
+
+Sub-nodes
+=========
+Required properties:
+- compatible : shall include one of the following
+	"arm,scpi-dvfs-clocks" - all the clocks that are variable and index based.
+		These clocks don't provide an entire range of values between the
+		limits but only discrete points within the range. The firmware
+		provides the mapping for each such operating frequency and the
+		index associated with it. The firmware also manages the
+		voltage scaling appropriately with the clock scaling.
+	"arm,scpi-variable-clocks" - all the clocks that are variable and provide full
+		range within the specified range. The firmware provides the
+		range of values within a specified range.
+
+Other required properties for all clocks(all from common clock binding):
+- #clock-cells : should be set to 1 as each of the SCPI clocks have multiple
+	outputs.
+- clock-output-names : shall be the corresponding names of the outputs.
+- clock-indices: The identifying number for the clocks(i.e.clock_id) in the
+	node. It can be non linear and hence provide the mapping of identifiers
+	into the clock-output-names array.
+
+SRAM and Shared Memory for SCPI
+-------------------------------
+
+A small area of SRAM is reserved for SCPI communication between application
+processors and SCP.
+
+Required properties:
+- compatible : should be "arm,juno-sram-ns" for Non-secure SRAM on Juno
+
+The rest of the properties should follow the generic mmio-sram description
+found in ../../misc/sysram.txt
+
+Each sub-node represents the reserved area for SCPI.
+
+Required sub-node properties:
+- reg : The base offset and size of the reserved area with the SRAM
+- compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
+	       shared memory on Juno platforms
+
+[0] http://community.arm.com/servlet/JiveServlet/download/8401-45-18326/DUI0922B_scp_message_interface.pdf
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Example:
+
+sram: sram@50000000 {
+	compatible = "arm,juno-sram-ns", "mmio-sram";
+	reg = <0x0 0x50000000 0x0 0x10000>;
+
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges = <0 0x0 0x50000000 0x10000>;
+
+	cpu_scp_lpri: scp-shmem@0 {
+		compatible = "arm,juno-scp-shmem";
+		reg = <0x0 0x200>;
+	};
+
+	cpu_scp_hpri: scp-shmem@200 {
+		compatible = "arm,juno-scp-shmem";
+		reg = <0x200 0x200>;
+	};
+};
+
+mailbox: mailbox0@40000000 {
+	....
+	#mbox-cells = <1>;
+};
+
+scpi_protocol: scpi@2e000000 {
+	compatible = "arm,scpi";
+	mboxes = <&mailbox 0 &mailbox 1>;
+	shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
+
+	clocks {
+		compatible = "arm,scpi-clocks";
+
+		scpi_dvfs: scpi_clocks@0 {
+			compatible = "arm,scpi-dvfs-clocks";
+			#clock-cells = <1>;
+			clock-indices = <0>, <1>, <2>;
+			clock-output-names = "atlclk", "aplclk","gpuclk";
+		};
+		scpi_clk: scpi_clocks@3 {
+			compatible = "arm,scpi-variable-clocks";
+			#clock-cells = <1>;
+			clock-indices = <3>, <4>;
+			clock-output-names = "pxlclk0", "pxlclk1";
+		};
+	};
+};
+
+cpu@0 {
+	...
+	reg = <0 0>;
+	clocks = <&scpi_dvfs 0>;
+};
+
+hdlcd@7ff60000 {
+	...
+	reg = <0 0x7ff60000 0 0x1000>;
+	clocks = <&scpi_clk 4>;
+};
+
+In the above example, the #clock-cells is set to 1 as required.
+scpi_dvfs has 3 output clocks namely: vbig, vlittle and vgpu with 0, 1
+and 2 as clock-indices. scpi_clk has 2 output clocks namely: pxlclk0 and
+pxlclk1 with 3 and 4 as clock-indices.
+
+The first consumer in the example is cpu@0 and it has '0' as the clock
+specifier which points to the first entry in the output clocks of
+scpi_dvfs i.e. "atlclk".
+
+Similarly the second example is hdlcd@7ff60000 and it has pxlclk1 as input
+clock. '4' in the clock specifier here points to the second entry
+in the output clocks of scpi_clocks  i.e. "pxlclk1"
diff --git a/MAINTAINERS b/MAINTAINERS
index a2264167791a..9351b62dbbd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8955,6 +8955,12 @@ W:	http://www.sunplus.com
 S:	Supported
 F:	arch/score/
 
+SYSTEM CONTROL & POWER INTERFACE (SCPI) Message Protocol drivers
+M:	Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
+L:	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/arm/arm,scpi.txt
+
 SCSI CDROM DRIVER
 M:	Jens Axboe <axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
 L:	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
-- 
1.9.1

--
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

[PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

This patch adds support for System Control and Power Interface (SCPI)
Message Protocol used between the Application Cores(AP) and the System
Control Processor(SCP). The MHU peripheral provides a mechanism for
inter-processor communication between SCP's M3 processor and AP.

SCP offers control and management of the core/cluster power states,
various power domain DVFS including the core/cluster, certain system
clocks configuration, thermal sensors and many others.

This protocol driver provides interface for all the client drivers using
SCPI to make use of the features offered by the SCP.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Reviewed-by: Jon Medhurst (Tixy) <tixy@linaro.org>
Cc: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 MAINTAINERS                   |   2 +
 drivers/firmware/Kconfig      |  19 ++
 drivers/firmware/Makefile     |   1 +
 drivers/firmware/arm_scpi.c   | 711 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/scpi_protocol.h |  61 ++++
 5 files changed, 794 insertions(+)
 create mode 100644 drivers/firmware/arm_scpi.c
 create mode 100644 include/linux/scpi_protocol.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 9351b62dbbd7..be511dd69ae4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8960,6 +8960,8 @@ M:	Sudeep Holla <sudeep.holla@arm.com>
 L:	linux-arm-kernel@lists.infradead.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/arm/arm,scpi.txt
+F:	drivers/firmware/arm_scpi.c
+F:	include/linux/scpi_protocol.h
 
 SCSI CDROM DRIVER
 M:	Jens Axboe <axboe@kernel.dk>
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 99c69a3205c4..fc40751a1a1f 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -136,6 +136,25 @@ config QCOM_SCM
 	bool
 	depends on ARM || ARM64
 
+config ARM_SCPI_PROTOCOL
+	tristate "ARM System Control and Power Interface (SCPI) Message Protocol"
+	depends on ARM_MHU
+	help
+	  System Control and Power Interface (SCPI) Message Protocol is
+	  defined for the purpose of communication between the Application
+	  Cores(AP) and the System Control Processor(SCP). The MHU peripheral
+	  provides a mechanism for inter-processor communication between SCP
+	  and AP.
+
+	  SCP controls most of the power managament on the Application
+	  Processors. It offers control and management of: the core/cluster
+	  power states, various power domain DVFS including the core/cluster,
+	  certain system clocks configuration, thermal sensors and many
+	  others.
+
+	  This protocol library provides interface for all the client drivers
+	  making use of the features offered by the SCP.
+
 source "drivers/firmware/broadcom/Kconfig"
 source "drivers/firmware/google/Kconfig"
 source "drivers/firmware/efi/Kconfig"
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 4a4b897f9314..85f1032c8898 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -1,6 +1,7 @@
 #
 # Makefile for the linux kernel.
 #
+obj-$(CONFIG_ARM_SCPI_PROTOCOL)	+= arm_scpi.o
 obj-$(CONFIG_DMI)		+= dmi_scan.o
 obj-$(CONFIG_DMI_SYSFS)		+= dmi-sysfs.o
 obj-$(CONFIG_EDD)		+= edd.o
diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
new file mode 100644
index 000000000000..5e3898f24b96
--- /dev/null
+++ b/drivers/firmware/arm_scpi.c
@@ -0,0 +1,711 @@
+/*
+ * System Control and Power Interface (SCPI) Message Protocol driver
+ *
+ * SCPI Message Protocol is used between the System Control Processor(SCP)
+ * and the Application Processors(AP). The Message Handling Unit(MHU)
+ * provides a mechanism for inter-processor communication between SCP's
+ * Cortex M3 and AP.
+ *
+ * SCP offers control and management of the core/cluster power states,
+ * various power domain DVFS including the core/cluster, certain system
+ * clocks configuration, thermal sensors and many others.
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/bitmap.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/printk.h>
+#include <linux/scpi_protocol.h>
+#include <linux/slab.h>
+#include <linux/sort.h>
+#include <linux/spinlock.h>
+
+#define CMD_ID_SHIFT		0
+#define CMD_ID_MASK		0x7f
+#define CMD_TOKEN_ID_SHIFT	8
+#define CMD_TOKEN_ID_MASK	0xff
+#define CMD_DATA_SIZE_SHIFT	16
+#define CMD_DATA_SIZE_MASK	0x1ff
+#define PACK_SCPI_CMD(cmd_id, tx_sz)			\
+	((((cmd_id) & CMD_ID_MASK) << CMD_ID_SHIFT) |	\
+	(((tx_sz) & CMD_DATA_SIZE_MASK) << CMD_DATA_SIZE_SHIFT))
+#define ADD_SCPI_TOKEN(cmd, token)			\
+	((cmd) |= (((token) & CMD_TOKEN_ID_MASK) << CMD_TOKEN_ID_SHIFT))
+
+#define CMD_SIZE(cmd)	(((cmd) >> CMD_DATA_SIZE_SHIFT) & CMD_DATA_SIZE_MASK)
+#define CMD_UNIQ_MASK	(CMD_TOKEN_ID_MASK << CMD_TOKEN_ID_SHIFT | CMD_ID_MASK)
+#define CMD_XTRACT_UNIQ(cmd)	((cmd) & CMD_UNIQ_MASK)
+
+#define SCPI_SLOT		0
+
+#define MAX_DVFS_DOMAINS	8
+#define MAX_DVFS_OPPS		8
+#define DVFS_LATENCY(hdr)	(le32_to_cpu(hdr) >> 16)
+#define DVFS_OPP_COUNT(hdr)	((le32_to_cpu(hdr) >> 8) & 0xff)
+
+#define PROTOCOL_REV_MINOR_BITS	16
+#define PROTOCOL_REV_MINOR_MASK	((1U << PROTOCOL_REV_MINOR_BITS) - 1)
+#define PROTOCOL_REV_MAJOR(x)	((x) >> PROTOCOL_REV_MINOR_BITS)
+#define PROTOCOL_REV_MINOR(x)	((x) & PROTOCOL_REV_MINOR_MASK)
+
+#define FW_REV_MAJOR_BITS	24
+#define FW_REV_MINOR_BITS	16
+#define FW_REV_PATCH_MASK	((1U << FW_REV_MINOR_BITS) - 1)
+#define FW_REV_MINOR_MASK	((1U << FW_REV_MAJOR_BITS) - 1)
+#define FW_REV_MAJOR(x)		((x) >> FW_REV_MAJOR_BITS)
+#define FW_REV_MINOR(x)		(((x) & FW_REV_MINOR_MASK) >> FW_REV_MINOR_BITS)
+#define FW_REV_PATCH(x)		((x) & FW_REV_PATCH_MASK)
+
+#define MAX_RX_TIMEOUT		(msecs_to_jiffies(20))
+
+enum scpi_error_codes {
+	SCPI_SUCCESS = 0, /* Success */
+	SCPI_ERR_PARAM = 1, /* Invalid parameter(s) */
+	SCPI_ERR_ALIGN = 2, /* Invalid alignment */
+	SCPI_ERR_SIZE = 3, /* Invalid size */
+	SCPI_ERR_HANDLER = 4, /* Invalid handler/callback */
+	SCPI_ERR_ACCESS = 5, /* Invalid access/permission denied */
+	SCPI_ERR_RANGE = 6, /* Value out of range */
+	SCPI_ERR_TIMEOUT = 7, /* Timeout has occurred */
+	SCPI_ERR_NOMEM = 8, /* Invalid memory area or pointer */
+	SCPI_ERR_PWRSTATE = 9, /* Invalid power state */
+	SCPI_ERR_SUPPORT = 10, /* Not supported or disabled */
+	SCPI_ERR_DEVICE = 11, /* Device error */
+	SCPI_ERR_BUSY = 12, /* Device busy */
+	SCPI_ERR_MAX
+};
+
+enum scpi_std_cmd {
+	SCPI_CMD_INVALID		= 0x00,
+	SCPI_CMD_SCPI_READY		= 0x01,
+	SCPI_CMD_SCPI_CAPABILITIES	= 0x02,
+	SCPI_CMD_SET_CSS_PWR_STATE	= 0x03,
+	SCPI_CMD_GET_CSS_PWR_STATE	= 0x04,
+	SCPI_CMD_SET_SYS_PWR_STATE	= 0x05,
+	SCPI_CMD_SET_CPU_TIMER		= 0x06,
+	SCPI_CMD_CANCEL_CPU_TIMER	= 0x07,
+	SCPI_CMD_DVFS_CAPABILITIES	= 0x08,
+	SCPI_CMD_GET_DVFS_INFO		= 0x09,
+	SCPI_CMD_SET_DVFS		= 0x0a,
+	SCPI_CMD_GET_DVFS		= 0x0b,
+	SCPI_CMD_GET_DVFS_STAT		= 0x0c,
+	SCPI_CMD_CLOCK_CAPABILITIES	= 0x0d,
+	SCPI_CMD_GET_CLOCK_INFO		= 0x0e,
+	SCPI_CMD_SET_CLOCK_VALUE	= 0x0f,
+	SCPI_CMD_GET_CLOCK_VALUE	= 0x10,
+	SCPI_CMD_PSU_CAPABILITIES	= 0x11,
+	SCPI_CMD_GET_PSU_INFO		= 0x12,
+	SCPI_CMD_SET_PSU		= 0x13,
+	SCPI_CMD_GET_PSU		= 0x14,
+	SCPI_CMD_SENSOR_CAPABILITIES	= 0x15,
+	SCPI_CMD_SENSOR_INFO		= 0x16,
+	SCPI_CMD_SENSOR_VALUE		= 0x17,
+	SCPI_CMD_SENSOR_CFG_PERIODIC	= 0x18,
+	SCPI_CMD_SENSOR_CFG_BOUNDS	= 0x19,
+	SCPI_CMD_SENSOR_ASYNC_VALUE	= 0x1a,
+	SCPI_CMD_SET_DEVICE_PWR_STATE	= 0x1b,
+	SCPI_CMD_GET_DEVICE_PWR_STATE	= 0x1c,
+	SCPI_CMD_COUNT
+};
+
+struct scpi_xfer {
+	u32 slot; /* has to be first element */
+	u32 cmd;
+	u32 status;
+	const void *tx_buf;
+	void *rx_buf;
+	unsigned int tx_len;
+	unsigned int rx_len;
+	struct list_head node;
+	struct completion done;
+};
+
+struct scpi_chan {
+	struct mbox_client cl;
+	struct mbox_chan *chan;
+	void __iomem *tx_payload;
+	void __iomem *rx_payload;
+	struct list_head rx_pending;
+	struct list_head xfers_list;
+	struct scpi_xfer *xfers;
+	spinlock_t rx_lock; /* locking for the rx pending list */
+	struct mutex xfers_lock;
+	u8 token;
+};
+
+struct scpi_drvinfo {
+	u32 protocol_version;
+	u32 firmware_version;
+	int num_chans;
+	atomic_t next_chan;
+	struct scpi_ops *scpi_ops;
+	struct scpi_chan *channels;
+	struct scpi_dvfs_info *dvfs[MAX_DVFS_DOMAINS];
+};
+
+/*
+ * The SCP firmware only executes in little-endian mode, so any buffers
+ * shared through SCPI should have their contents converted to little-endian
+ */
+struct scpi_shared_mem {
+	__le32 command;
+	__le32 status;
+	u8 payload[0];
+} __packed;
+
+struct scp_capabilities {
+	__le32 protocol_version;
+	__le32 event_version;
+	__le32 platform_version;
+	__le32 commands[4];
+} __packed;
+
+struct clk_get_info {
+	__le16 id;
+	__le16 flags;
+	__le32 min_rate;
+	__le32 max_rate;
+	u8 name[20];
+} __packed;
+
+struct clk_get_value {
+	__le32 rate;
+} __packed;
+
+struct clk_set_value {
+	__le16 id;
+	__le16 reserved;
+	__le32 rate;
+} __packed;
+
+struct dvfs_info {
+	__le32 header;
+	struct {
+		__le32 freq;
+		__le32 m_volt;
+	} opps[MAX_DVFS_OPPS];
+} __packed;
+
+struct dvfs_get {
+	u8 index;
+} __packed;
+
+struct dvfs_set {
+	u8 domain;
+	u8 index;
+} __packed;
+
+static struct scpi_drvinfo *scpi_info;
+
+static int scpi_linux_errmap[SCPI_ERR_MAX] = {
+	/* better than switch case as long as return value is continuous */
+	0, /* SCPI_SUCCESS */
+	-EINVAL, /* SCPI_ERR_PARAM */
+	-ENOEXEC, /* SCPI_ERR_ALIGN */
+	-EMSGSIZE, /* SCPI_ERR_SIZE */
+	-EINVAL, /* SCPI_ERR_HANDLER */
+	-EACCES, /* SCPI_ERR_ACCESS */
+	-ERANGE, /* SCPI_ERR_RANGE */
+	-ETIMEDOUT, /* SCPI_ERR_TIMEOUT */
+	-ENOMEM, /* SCPI_ERR_NOMEM */
+	-EINVAL, /* SCPI_ERR_PWRSTATE */
+	-EOPNOTSUPP, /* SCPI_ERR_SUPPORT */
+	-EIO, /* SCPI_ERR_DEVICE */
+	-EBUSY, /* SCPI_ERR_BUSY */
+};
+
+static inline int scpi_to_linux_errno(int errno)
+{
+	if (errno >= SCPI_SUCCESS && errno < SCPI_ERR_MAX)
+		return scpi_linux_errmap[errno];
+	return -EIO;
+}
+
+static void scpi_process_cmd(struct scpi_chan *ch, u32 cmd)
+{
+	unsigned long flags;
+	struct scpi_xfer *t, *match = NULL;
+
+	spin_lock_irqsave(&ch->rx_lock, flags);
+	if (list_empty(&ch->rx_pending)) {
+		spin_unlock_irqrestore(&ch->rx_lock, flags);
+		return;
+	}
+
+	list_for_each_entry(t, &ch->rx_pending, node)
+		if (CMD_XTRACT_UNIQ(t->cmd) == CMD_XTRACT_UNIQ(cmd)) {
+			list_del(&t->node);
+			match = t;
+			break;
+		}
+	/* check if wait_for_completion is in progress or timed-out */
+	if (match && !completion_done(&match->done)) {
+		struct scpi_shared_mem *mem = ch->rx_payload;
+		unsigned int len = min(match->rx_len, CMD_SIZE(cmd));
+
+		match->status = le32_to_cpu(mem->status);
+		memcpy_fromio(match->rx_buf, mem->payload, len);
+		if (match->rx_len > len)
+			memset(match->rx_buf + len, 0, match->rx_len - len);
+		complete(&match->done);
+	}
+	spin_unlock_irqrestore(&ch->rx_lock, flags);
+}
+
+static void scpi_handle_remote_msg(struct mbox_client *c, void *msg)
+{
+	struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
+	struct scpi_shared_mem *mem = ch->rx_payload;
+	u32 cmd = le32_to_cpu(mem->command);
+
+	scpi_process_cmd(ch, cmd);
+}
+
+static void scpi_tx_prepare(struct mbox_client *c, void *msg)
+{
+	unsigned long flags;
+	struct scpi_xfer *t = msg;
+	struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
+	struct scpi_shared_mem *mem = (struct scpi_shared_mem *)ch->tx_payload;
+
+	if (t->tx_buf)
+		memcpy_toio(mem->payload, t->tx_buf, t->tx_len);
+	if (t->rx_buf) {
+		if (!(++ch->token))
+			++ch->token;
+		ADD_SCPI_TOKEN(t->cmd, ch->token);
+		spin_lock_irqsave(&ch->rx_lock, flags);
+		list_add_tail(&t->node, &ch->rx_pending);
+		spin_unlock_irqrestore(&ch->rx_lock, flags);
+	}
+	mem->command = cpu_to_le32(t->cmd);
+}
+
+static struct scpi_xfer *get_scpi_xfer(struct scpi_chan *ch)
+{
+	struct scpi_xfer *t;
+
+	mutex_lock(&ch->xfers_lock);
+	if (list_empty(&ch->xfers_list)) {
+		mutex_unlock(&ch->xfers_lock);
+		return NULL;
+	}
+	t = list_first_entry(&ch->xfers_list, struct scpi_xfer, node);
+	list_del(&t->node);
+	mutex_unlock(&ch->xfers_lock);
+	return t;
+}
+
+static void put_scpi_xfer(struct scpi_xfer *t, struct scpi_chan *ch)
+{
+	mutex_lock(&ch->xfers_lock);
+	list_add_tail(&t->node, &ch->xfers_list);
+	mutex_unlock(&ch->xfers_lock);
+}
+
+static int scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len,
+			     void *rx_buf, unsigned int rx_len)
+{
+	int ret;
+	u8 chan;
+	struct scpi_xfer *msg;
+	struct scpi_chan *scpi_chan;
+
+	chan = atomic_inc_return(&scpi_info->next_chan) % scpi_info->num_chans;
+	scpi_chan = scpi_info->channels + chan;
+
+	msg = get_scpi_xfer(scpi_chan);
+	if (!msg)
+		return -ENOMEM;
+
+	msg->slot = BIT(SCPI_SLOT);
+	msg->cmd = PACK_SCPI_CMD(cmd, tx_len);
+	msg->tx_buf = tx_buf;
+	msg->tx_len = tx_len;
+	msg->rx_buf = rx_buf;
+	msg->rx_len = rx_len;
+	init_completion(&msg->done);
+
+	ret = mbox_send_message(scpi_chan->chan, msg);
+	if (ret < 0 || !rx_buf)
+		goto out;
+
+	if (!wait_for_completion_timeout(&msg->done, MAX_RX_TIMEOUT))
+		ret = -ETIMEDOUT;
+	else
+		/* first status word */
+		ret = le32_to_cpu(msg->status);
+out:
+	if (ret < 0 && rx_buf) /* remove entry from the list if timed-out */
+		scpi_process_cmd(scpi_chan, msg->cmd);
+
+	put_scpi_xfer(msg, scpi_chan);
+	/* SCPI error codes > 0, translate them to Linux scale*/
+	return ret > 0 ? scpi_to_linux_errno(ret) : ret;
+}
+
+static u32 scpi_get_version(void)
+{
+	return scpi_info->protocol_version;
+}
+
+static int
+scpi_clk_get_range(u16 clk_id, unsigned long *min, unsigned long *max)
+{
+	int ret;
+	struct clk_get_info clk;
+	__le16 le_clk_id = cpu_to_le16(clk_id);
+
+	ret = scpi_send_message(SCPI_CMD_GET_CLOCK_INFO, &le_clk_id,
+				sizeof(le_clk_id), &clk, sizeof(clk));
+	if (!ret) {
+		*min = le32_to_cpu(clk.min_rate);
+		*max = le32_to_cpu(clk.max_rate);
+	}
+	return ret;
+}
+
+static unsigned long scpi_clk_get_val(u16 clk_id)
+{
+	int ret;
+	struct clk_get_value clk;
+	__le16 le_clk_id = cpu_to_le16(clk_id);
+
+	ret = scpi_send_message(SCPI_CMD_GET_CLOCK_VALUE, &le_clk_id,
+				sizeof(le_clk_id), &clk, sizeof(clk));
+	return ret ? ret : le32_to_cpu(clk.rate);
+}
+
+static int scpi_clk_set_val(u16 clk_id, unsigned long rate)
+{
+	int stat;
+	struct clk_set_value clk = {
+		.id = cpu_to_le16(clk_id),
+		.rate = cpu_to_le32(rate)
+	};
+
+	return scpi_send_message(SCPI_CMD_SET_CLOCK_VALUE, &clk, sizeof(clk),
+				 &stat, sizeof(stat));
+}
+
+static int scpi_dvfs_get_idx(u8 domain)
+{
+	int ret;
+	struct dvfs_get dvfs;
+
+	ret = scpi_send_message(SCPI_CMD_GET_DVFS, &domain, sizeof(domain),
+				&dvfs, sizeof(dvfs));
+	return ret ? ret : dvfs.index;
+}
+
+static int scpi_dvfs_set_idx(u8 domain, u8 index)
+{
+	int stat;
+	struct dvfs_set dvfs = {domain, index};
+
+	return scpi_send_message(SCPI_CMD_SET_DVFS, &dvfs, sizeof(dvfs),
+				 &stat, sizeof(stat));
+}
+
+static int opp_cmp_func(const void *opp1, const void *opp2)
+{
+	const struct scpi_opp *t1 = opp1, *t2 = opp2;
+
+	return t1->freq - t2->freq;
+}
+
+static struct scpi_dvfs_info *scpi_dvfs_get_info(u8 domain)
+{
+	struct scpi_dvfs_info *info;
+	struct scpi_opp *opp;
+	struct dvfs_info buf;
+	int ret, i;
+
+	if (domain >= MAX_DVFS_DOMAINS)
+		return ERR_PTR(-EINVAL);
+
+	if (scpi_info->dvfs[domain])	/* data already populated */
+		return scpi_info->dvfs[domain];
+
+	ret = scpi_send_message(SCPI_CMD_GET_DVFS_INFO, &domain, sizeof(domain),
+				&buf, sizeof(buf));
+
+	if (ret)
+		return ERR_PTR(ret);
+
+	info = kmalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return ERR_PTR(-ENOMEM);
+
+	info->count = DVFS_OPP_COUNT(buf.header);
+	info->latency = DVFS_LATENCY(buf.header) * 1000; /* uS to nS */
+
+	info->opps = kcalloc(info->count, sizeof(*opp), GFP_KERNEL);
+	if (!info->opps) {
+		kfree(info);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	for (i = 0, opp = info->opps; i < info->count; i++, opp++) {
+		opp->freq = le32_to_cpu(buf.opps[i].freq);
+		opp->m_volt = le32_to_cpu(buf.opps[i].m_volt);
+	}
+
+	sort(info->opps, info->count, sizeof(*opp), opp_cmp_func, NULL);
+
+	scpi_info->dvfs[domain] = info;
+	return info;
+}
+
+static struct scpi_ops scpi_ops = {
+	.get_version = scpi_get_version,
+	.clk_get_range = scpi_clk_get_range,
+	.clk_get_val = scpi_clk_get_val,
+	.clk_set_val = scpi_clk_set_val,
+	.dvfs_get_idx = scpi_dvfs_get_idx,
+	.dvfs_set_idx = scpi_dvfs_set_idx,
+	.dvfs_get_info = scpi_dvfs_get_info,
+};
+
+struct scpi_ops *get_scpi_ops(void)
+{
+	return scpi_info ? scpi_info->scpi_ops : NULL;
+}
+EXPORT_SYMBOL_GPL(get_scpi_ops);
+
+static int scpi_init_versions(struct scpi_drvinfo *info)
+{
+	int ret;
+	struct scp_capabilities caps;
+
+	ret = scpi_send_message(SCPI_CMD_SCPI_CAPABILITIES, NULL, 0,
+				&caps, sizeof(caps));
+	if (!ret) {
+		info->protocol_version = le32_to_cpu(caps.protocol_version);
+		info->firmware_version = le32_to_cpu(caps.platform_version);
+	}
+	return ret;
+}
+
+static ssize_t protocol_version_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct scpi_drvinfo *scpi_info = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d.%d\n",
+		       PROTOCOL_REV_MAJOR(scpi_info->protocol_version),
+		       PROTOCOL_REV_MINOR(scpi_info->protocol_version));
+}
+static DEVICE_ATTR_RO(protocol_version);
+
+static ssize_t firmware_version_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct scpi_drvinfo *scpi_info = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d.%d.%d\n",
+		       FW_REV_MAJOR(scpi_info->firmware_version),
+		       FW_REV_MINOR(scpi_info->firmware_version),
+		       FW_REV_PATCH(scpi_info->firmware_version));
+}
+static DEVICE_ATTR_RO(firmware_version);
+
+static struct attribute *versions_attrs[] = {
+	&dev_attr_firmware_version.attr,
+	&dev_attr_protocol_version.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(versions);
+
+static void
+scpi_free_channels(struct device *dev, struct scpi_chan *pchan, int count)
+{
+	int i;
+
+	for (i = 0; i < count && pchan->chan; i++, pchan++) {
+		mbox_free_channel(pchan->chan);
+		devm_kfree(dev, pchan->xfers);
+		devm_iounmap(dev, pchan->rx_payload);
+	}
+}
+
+static int scpi_remove(struct platform_device *pdev)
+{
+	int i;
+	struct device *dev = &pdev->dev;
+	struct scpi_drvinfo *info = platform_get_drvdata(pdev);
+
+	scpi_info = NULL; /* stop exporting SCPI ops through get_scpi_ops */
+
+	of_platform_depopulate(dev);
+	sysfs_remove_groups(&dev->kobj, versions_groups);
+	scpi_free_channels(dev, info->channels, info->num_chans);
+	platform_set_drvdata(pdev, NULL);
+
+	for (i = 0; i < MAX_DVFS_DOMAINS && info->dvfs[i]; i++) {
+		kfree(info->dvfs[i]->opps);
+		kfree(info->dvfs[i]);
+	}
+	devm_kfree(dev, info->channels);
+	devm_kfree(dev, info);
+
+	return 0;
+}
+
+#define MAX_SCPI_XFERS		10
+static int scpi_alloc_xfer_list(struct device *dev, struct scpi_chan *ch)
+{
+	int i;
+	struct scpi_xfer *xfers;
+
+	xfers = devm_kzalloc(dev, MAX_SCPI_XFERS * sizeof(*xfers), GFP_KERNEL);
+	if (!xfers)
+		return -ENOMEM;
+
+	ch->xfers = xfers;
+	for (i = 0; i < MAX_SCPI_XFERS; i++, xfers++)
+		list_add_tail(&xfers->node, &ch->xfers_list);
+	return 0;
+}
+
+static int scpi_probe(struct platform_device *pdev)
+{
+	int count, idx, ret;
+	struct resource res;
+	struct scpi_chan *scpi_chan;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+
+	scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
+	if (!scpi_info)
+		return -ENOMEM;
+
+	count = of_count_phandle_with_args(np, "mboxes", "#mbox-cells");
+	if (count < 0) {
+		dev_err(dev, "no mboxes property in '%s'\n", np->full_name);
+		return -ENODEV;
+	}
+
+	scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan), GFP_KERNEL);
+	if (!scpi_chan)
+		return -ENOMEM;
+
+	for (idx = 0; idx < count; idx++) {
+		resource_size_t size;
+		struct scpi_chan *pchan = scpi_chan + idx;
+		struct mbox_client *cl = &pchan->cl;
+		struct device_node *shmem = of_parse_phandle(np, "shmem", idx);
+
+		if (of_address_to_resource(shmem, 0, &res)) {
+			dev_err(dev, "failed to get SCPI payload mem resource\n");
+			ret = -EINVAL;
+			goto err;
+		}
+
+		size = resource_size(&res);
+		pchan->rx_payload = devm_ioremap(dev, res.start, size);
+		if (!pchan->rx_payload) {
+			dev_err(dev, "failed to ioremap SCPI payload\n");
+			ret = -EADDRNOTAVAIL;
+			goto err;
+		}
+		pchan->tx_payload = pchan->rx_payload + (size >> 1);
+
+		cl->dev = dev;
+		cl->rx_callback = scpi_handle_remote_msg;
+		cl->tx_prepare = scpi_tx_prepare;
+		cl->tx_block = true;
+		cl->tx_tout = 50;
+		cl->knows_txdone = false; /* controller can ack */
+
+		INIT_LIST_HEAD(&pchan->rx_pending);
+		INIT_LIST_HEAD(&pchan->xfers_list);
+		spin_lock_init(&pchan->rx_lock);
+		mutex_init(&pchan->xfers_lock);
+
+		ret = scpi_alloc_xfer_list(dev, pchan);
+		if (!ret) {
+			pchan->chan = mbox_request_channel(cl, idx);
+			if (!IS_ERR(pchan->chan))
+				continue;
+			ret = PTR_ERR(pchan->chan);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "failed to get channel%d err %d\n",
+					idx, ret);
+		}
+err:
+		scpi_free_channels(dev, scpi_chan, idx);
+		scpi_info = NULL;
+		return ret;
+	}
+
+	scpi_info->channels = scpi_chan;
+	scpi_info->num_chans = count;
+	platform_set_drvdata(pdev, scpi_info);
+
+	ret = scpi_init_versions(scpi_info);
+	if (ret) {
+		dev_err(dev, "incorrect or no SCP firmware found\n");
+		scpi_remove(pdev);
+		return ret;
+	}
+
+	_dev_info(dev, "SCP Protocol %d.%d Firmware %d.%d.%d version\n",
+		  PROTOCOL_REV_MAJOR(scpi_info->protocol_version),
+		  PROTOCOL_REV_MINOR(scpi_info->protocol_version),
+		  FW_REV_MAJOR(scpi_info->firmware_version),
+		  FW_REV_MINOR(scpi_info->firmware_version),
+		  FW_REV_PATCH(scpi_info->firmware_version));
+	scpi_info->scpi_ops = &scpi_ops;
+
+	ret = sysfs_create_groups(&dev->kobj, versions_groups);
+	if (ret)
+		dev_err(dev, "unable to create sysfs version group\n");
+
+	return of_platform_populate(dev->of_node, NULL, NULL, dev);
+}
+
+static const struct of_device_id scpi_of_match[] = {
+	{.compatible = "arm,scpi"},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, scpi_of_match);
+
+static struct platform_driver scpi_driver = {
+	.driver = {
+		.name = "scpi_protocol",
+		.of_match_table = scpi_of_match,
+	},
+	.probe = scpi_probe,
+	.remove = scpi_remove,
+};
+module_platform_driver(scpi_driver);
+
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("ARM SCPI mailbox protocol driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/scpi_protocol.h b/include/linux/scpi_protocol.h
new file mode 100644
index 000000000000..e7169cd54e19
--- /dev/null
+++ b/include/linux/scpi_protocol.h
@@ -0,0 +1,61 @@
+/*
+ * SCPI Message Protocol driver header
+ *
+ * Copyright (C) 2014 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/types.h>
+
+struct scpi_opp {
+	u32 freq;
+	u32 m_volt;
+} __packed;
+
+struct scpi_dvfs_info {
+	unsigned int count;
+	unsigned int latency; /* in nanoseconds */
+	struct scpi_opp *opps;
+};
+
+/**
+ * struct scpi_ops - represents the various operations provided
+ *	by SCP through SCPI message protocol
+ * @get_version: returns the major and minor revision on the SCPI
+ *	message protocol
+ * @clk_get_range: gets clock range limit(min - max in Hz)
+ * @clk_get_val: gets clock value(in Hz)
+ * @clk_set_val: sets the clock value, setting to 0 will disable the
+ *	clock (if supported)
+ * @dvfs_get_idx: gets the Operating Point of the given power domain.
+ *	OPP is an index to the list return by @dvfs_get_info
+ * @dvfs_set_idx: sets the Operating Point of the given power domain.
+ *	OPP is an index to the list return by @dvfs_get_info
+ * @dvfs_get_info: returns the DVFS capabilities of the given power
+ *	domain. It includes the OPP list and the latency information
+ */
+struct scpi_ops {
+	u32 (*get_version)(void);
+	int (*clk_get_range)(u16, unsigned long *, unsigned long *);
+	unsigned long (*clk_get_val)(u16);
+	int (*clk_set_val)(u16, unsigned long);
+	int (*dvfs_get_idx)(u8);
+	int (*dvfs_set_idx)(u8, u8);
+	struct scpi_dvfs_info *(*dvfs_get_info)(u8);
+};
+
+#if IS_ENABLED(CONFIG_ARM_SCPI_PROTOCOL)
+struct scpi_ops *get_scpi_ops(void);
+#else
+static inline struct scpi_ops *get_scpi_ops(void) { return NULL; }
+#endif
-- 
1.9.1

[PATCH v5 3/8] clk: add support for clocks provided by SCP(System Control Processor)

[Sudeep Holla]

On some ARM based systems, a separate Cortex-M based System Control
Processor(SCP) provides the overall power, clock, reset and system
control. System Control and Power Interface(SCPI) Message Protocol
is defined for the communication between the Application Cores(AP)
and the SCP.

This patch adds support for the clocks provided by SCP using SCPI
protocol.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Cc: Mike Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Jon Medhurst (Tixy) <tixy@linaro.org>
Cc: linux-clk@vger.kernel.org
---
 MAINTAINERS            |   1 +
 drivers/clk/Kconfig    |  10 ++
 drivers/clk/Makefile   |   1 +
 drivers/clk/clk-scpi.c | 311 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 323 insertions(+)
 create mode 100644 drivers/clk/clk-scpi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index be511dd69ae4..16a7612fee4a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8960,6 +8960,7 @@ M:	Sudeep Holla <sudeep.holla@arm.com>
 L:	linux-arm-kernel@lists.infradead.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/arm/arm,scpi.txt
+F:	drivers/clk/clk-scpi.c
 F:	drivers/firmware/arm_scpi.c
 F:	include/linux/scpi_protocol.h
 
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 42f7120ca9ce..4aec54b90331 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -59,6 +59,16 @@ config COMMON_CLK_RK808
 	  clocked at 32KHz each. Clkout1 is always on, Clkout2 can off
 	  by control register.
 
+config COMMON_CLK_SCPI
+	tristate "Clock driver controlled via SCPI interface"
+	depends on ARM_SCPI_PROTOCOL || COMPILE_TEST
+	  ---help---
+	  This driver provides support for clocks that are controlled
+	  by firmware that implements the SCPI interface.
+
+	  This driver uses SCPI Message Protocol to interact with the
+	  firmware providing all the clock controls.
+
 config COMMON_CLK_SI5351
 	tristate "Clock driver for SiLabs 5351A/B/C"
 	depends on I2C
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index c4cf075a2320..bef9c7064e5b 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_COMMON_CLK_PALMAS)		+= clk-palmas.o
 obj-$(CONFIG_CLK_QORIQ)			+= clk-qoriq.o
 obj-$(CONFIG_COMMON_CLK_RK808)		+= clk-rk808.o
 obj-$(CONFIG_COMMON_CLK_S2MPS11)	+= clk-s2mps11.o
+obj-$(CONFIG_COMMON_CLK_SCPI)           += clk-scpi.o
 obj-$(CONFIG_COMMON_CLK_SI5351)		+= clk-si5351.o
 obj-$(CONFIG_COMMON_CLK_SI570)		+= clk-si570.o
 obj-$(CONFIG_COMMON_CLK_CDCE925)	+= clk-cdce925.o
diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c
new file mode 100644
index 000000000000..3ce4b34a4128
--- /dev/null
+++ b/drivers/clk/clk-scpi.c
@@ -0,0 +1,311 @@
+/*
+ * System Control and Power Interface (SCPI) Protocol based clock driver
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/scpi_protocol.h>
+
+struct scpi_clk {
+	u32 id;
+	struct clk_hw hw;
+	struct scpi_dvfs_info *info;
+	struct scpi_ops *scpi_ops;
+};
+
+#define to_scpi_clk(clk) container_of(clk, struct scpi_clk, hw)
+
+static unsigned long scpi_clk_recalc_rate(struct clk_hw *hw,
+					  unsigned long parent_rate)
+{
+	struct scpi_clk *clk = to_scpi_clk(hw);
+
+	return clk->scpi_ops->clk_get_val(clk->id);
+}
+
+static long scpi_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long *parent_rate)
+{
+	/*
+	 * We can't figure out what rate it will be, so just return the
+	 * rate back to the caller. scpi_clk_recalc_rate() will be called
+	 * after the rate is set and we'll know what rate the clock is
+	 * running at then.
+	 */
+	return rate;
+}
+
+static int scpi_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+			     unsigned long parent_rate)
+{
+	struct scpi_clk *clk = to_scpi_clk(hw);
+
+	return clk->scpi_ops->clk_set_val(clk->id, rate);
+}
+
+static const struct clk_ops scpi_clk_ops = {
+	.recalc_rate = scpi_clk_recalc_rate,
+	.round_rate = scpi_clk_round_rate,
+	.set_rate = scpi_clk_set_rate,
+};
+
+/* find closest match to given frequency in OPP table */
+static int __scpi_dvfs_round_rate(struct scpi_clk *clk, unsigned long rate)
+{
+	int idx;
+	u32 fmin = 0, fmax = ~0, ftmp;
+	const struct scpi_opp *opp = clk->info->opps;
+
+	for (idx = 0; idx < clk->info->count; idx++, opp++) {
+		ftmp = opp->freq;
+		if (ftmp >= (u32)rate) {
+			if (ftmp <= fmax)
+				fmax = ftmp;
+			break;
+		} else if (ftmp >= fmin) {
+			fmin = ftmp;
+		}
+	}
+	return fmax != ~0 ? fmax : fmin;
+}
+
+static unsigned long scpi_dvfs_recalc_rate(struct clk_hw *hw,
+					   unsigned long parent_rate)
+{
+	struct scpi_clk *clk = to_scpi_clk(hw);
+	int idx = clk->scpi_ops->dvfs_get_idx(clk->id);
+	const struct scpi_opp *opp;
+
+	if (idx < 0)
+		return 0;
+
+	opp = clk->info->opps + idx;
+	return opp->freq;
+}
+
+static long scpi_dvfs_round_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long *parent_rate)
+{
+	struct scpi_clk *clk = to_scpi_clk(hw);
+
+	return __scpi_dvfs_round_rate(clk, rate);
+}
+
+static int __scpi_find_dvfs_index(struct scpi_clk *clk, unsigned long rate)
+{
+	int idx, max_opp = clk->info->count;
+	const struct scpi_opp *opp = clk->info->opps;
+
+	for (idx = 0; idx < max_opp; idx++, opp++)
+		if (opp->freq == rate)
+			return idx;
+	return -EINVAL;
+}
+
+static int scpi_dvfs_set_rate(struct clk_hw *hw, unsigned long rate,
+			      unsigned long parent_rate)
+{
+	struct scpi_clk *clk = to_scpi_clk(hw);
+	int ret = __scpi_find_dvfs_index(clk, rate);
+
+	if (ret < 0)
+		return ret;
+	return clk->scpi_ops->dvfs_set_idx(clk->id, (u8)ret);
+}
+
+static const struct clk_ops scpi_dvfs_ops = {
+	.recalc_rate = scpi_dvfs_recalc_rate,
+	.round_rate = scpi_dvfs_round_rate,
+	.set_rate = scpi_dvfs_set_rate,
+};
+
+static const struct of_device_id scpi_clk_match[] = {
+	{ .compatible = "arm,scpi-dvfs-clocks", .data = &scpi_dvfs_ops, },
+	{ .compatible = "arm,scpi-variable-clocks", .data = &scpi_clk_ops, },
+	{}
+};
+
+static struct clk *
+scpi_clk_ops_init(struct device *dev, const struct of_device_id *match,
+		  struct scpi_clk *sclk, const char *name)
+{
+	struct clk_init_data init;
+	struct clk *clk;
+	unsigned long min = 0, max = 0;
+
+	init.name = name;
+	init.flags = CLK_IS_ROOT;
+	init.num_parents = 0;
+	init.ops = match->data;
+	sclk->hw.init = &init;
+	sclk->scpi_ops = get_scpi_ops();
+
+	if (init.ops == &scpi_dvfs_ops) {
+		sclk->info = sclk->scpi_ops->dvfs_get_info(sclk->id);
+		if (IS_ERR(sclk->info))
+			return NULL;
+	} else if (init.ops == &scpi_clk_ops) {
+		if (sclk->scpi_ops->clk_get_range(sclk->id, &min, &max) || !max)
+			return NULL;
+	} else {
+		return NULL;
+	}
+
+	clk = devm_clk_register(dev, &sclk->hw);
+	if (!IS_ERR(clk) && max)
+		clk_set_rate_range(clk, min, max);
+	return clk;
+}
+
+struct scpi_clk_data {
+	struct scpi_clk **clk;
+	unsigned int clk_num;
+};
+
+static struct clk *
+scpi_of_clk_src_get(struct of_phandle_args *clkspec, void *data)
+{
+	struct scpi_clk *sclk;
+	struct scpi_clk_data *clk_data = data;
+	unsigned int idx = clkspec->args[0], count;
+
+	for (count = 0; count < clk_data->clk_num; count++) {
+		sclk = clk_data->clk[count];
+		if (idx == sclk->id)
+			return sclk->hw.clk;
+	}
+
+	return ERR_PTR(-EINVAL);
+}
+
+static int scpi_clk_add(struct device *dev, struct device_node *np,
+			const struct of_device_id *match)
+{
+	struct clk **clks;
+	int idx, count;
+	struct scpi_clk_data *clk_data;
+
+	count = of_property_count_strings(np, "clock-output-names");
+	if (count < 0) {
+		dev_err(dev, "%s: invalid clock output count\n", np->name);
+		return -EINVAL;
+	}
+
+	clk_data = devm_kmalloc(dev, sizeof(*clk_data), GFP_KERNEL);
+	if (!clk_data)
+		return -ENOMEM;
+
+	clk_data->clk_num = count;
+	clk_data->clk = devm_kcalloc(dev, count, sizeof(*clk_data->clk),
+				     GFP_KERNEL);
+	if (!clk_data->clk)
+		return -ENOMEM;
+
+	clks = devm_kcalloc(dev, count, sizeof(*clks), GFP_KERNEL);
+	if (!clks)
+		return -ENOMEM;
+
+	for (idx = 0; idx < count; idx++) {
+		struct scpi_clk *sclk;
+		const char *name;
+		u32 val;
+
+		sclk = devm_kzalloc(dev, sizeof(*sclk), GFP_KERNEL);
+		if (!sclk)
+			return -ENOMEM;
+
+		if (of_property_read_string_index(np, "clock-output-names",
+						  idx, &name)) {
+			dev_err(dev, "invalid clock name @ %s\n", np->name);
+			return -EINVAL;
+		}
+
+		if (of_property_read_u32_index(np, "clock-indices",
+					       idx, &val)) {
+			dev_err(dev, "invalid clock index @ %s\n", np->name);
+			return -EINVAL;
+		}
+
+		sclk->id = val;
+
+		clks[idx] = scpi_clk_ops_init(dev, match, sclk, name);
+		if (IS_ERR_OR_NULL(clks[idx]))
+			dev_err(dev, "failed to register clock '%s'\n", name);
+		else
+			dev_dbg(dev, "Registered clock '%s'\n", name);
+		clk_data->clk[idx] = sclk;
+	}
+
+	return of_clk_add_provider(np, scpi_of_clk_src_get, clk_data);
+}
+
+static int scpi_clocks_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *child, *np = dev->of_node;
+
+	for_each_available_child_of_node(np, child)
+		of_clk_del_provider(np);
+	return 0;
+}
+
+static int scpi_clocks_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct device *dev = &pdev->dev;
+	struct device_node *child, *np = dev->of_node;
+	const struct of_device_id *match;
+
+	if (!get_scpi_ops())
+		return -ENXIO;
+
+	for_each_available_child_of_node(np, child) {
+		match = of_match_node(scpi_clk_match, child);
+		if (!match)
+			continue;
+		ret = scpi_clk_add(dev, child, match);
+		if (ret) {
+			scpi_clocks_remove(pdev);
+			return ret;
+		}
+	}
+	return 0;
+}
+
+static const struct of_device_id scpi_clocks_ids[] = {
+	{ .compatible = "arm,scpi-clocks", },
+	{}
+};
+
+static struct platform_driver scpi_clocks_driver = {
+	.driver	= {
+		.name = "scpi_clocks",
+		.of_match_table = scpi_clocks_ids,
+	},
+	.probe = scpi_clocks_probe,
+	.remove = scpi_clocks_remove,
+};
+module_platform_driver(scpi_clocks_driver);
+
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("ARM SCPI clock driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH v5 4/8] clk: scpi: add support for cpufreq virtual device

[Sudeep Holla]

The clocks for the CPUs are provided by SCP and are managed by this
clock driver. So the cpufreq device needs to be added only after the
clock get registered and removed when this driver is unloaded.

This patch manages the cpufreq virtual device based on the clock
availability.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Mike Turquette <mturquette@baylibre.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Jon Medhurst (Tixy) <tixy@linaro.org>
Cc: linux-clk@vger.kernel.org
---
 drivers/clk/clk-scpi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c
index 3ce4b34a4128..b76db39ca671 100644
--- a/drivers/clk/clk-scpi.c
+++ b/drivers/clk/clk-scpi.c
@@ -34,6 +34,8 @@ struct scpi_clk {
 
 #define to_scpi_clk(clk) container_of(clk, struct scpi_clk, hw)
 
+static struct platform_device *cpufreq_dev;
+
 static unsigned long scpi_clk_recalc_rate(struct clk_hw *hw,
 					  unsigned long parent_rate)
 {
@@ -263,6 +265,11 @@ static int scpi_clocks_remove(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *child, *np = dev->of_node;
 
+	if (cpufreq_dev) {
+		platform_device_unregister(cpufreq_dev);
+		cpufreq_dev = NULL;
+	}
+
 	for_each_available_child_of_node(np, child)
 		of_clk_del_provider(np);
 	return 0;
@@ -288,6 +295,12 @@ static int scpi_clocks_probe(struct platform_device *pdev)
 			return ret;
 		}
 	}
+	/* Add the virtual cpufreq device */
+	cpufreq_dev = platform_device_register_simple("scpi-cpufreq",
+						      -1, NULL, 0);
+	if (!cpufreq_dev)
+		pr_warn("unable to register cpufreq device");
+
 	return 0;
 }
 
-- 
1.9.1

[PATCH v5 5/8] cpufreq: arm_big_little: add SCPI interface driver

[Sudeep Holla]

On some ARM based systems, a separate Cortex-M based System Control
Processor(SCP) provides the overall power, clock, reset and system
control including CPU DVFS. SCPI Message Protocol is used to
communicate with the SCPI.

This patch adds a interface driver for adding OPPs and registering
the arm_big_little cpufreq driver for such systems.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: linux-pm@vger.kernel.org
---
 MAINTAINERS                    |   1 +
 drivers/cpufreq/Kconfig.arm    |   9 +++
 drivers/cpufreq/Makefile       |   1 +
 drivers/cpufreq/scpi-cpufreq.c | 124 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 135 insertions(+)
 create mode 100644 drivers/cpufreq/scpi-cpufreq.c

Hi Rafael,

Viresh has already ACKed, since I plan to push this as a series via arm-soc,
can you provide your ACK if the patch looks fine to you ?

Regards,
Sudeep

diff --git a/MAINTAINERS b/MAINTAINERS
index 16a7612fee4a..74e5c80c7db9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8961,6 +8961,7 @@ L:	linux-arm-kernel@lists.infradead.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/arm/arm,scpi.txt
 F:	drivers/clk/clk-scpi.c
+F:	drivers/cpufreq/scpi-cpufreq.c
 F:	drivers/firmware/arm_scpi.c
 F:	include/linux/scpi_protocol.h
 
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index cc8a71c267b8..c46b495169af 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -24,6 +24,15 @@ config ARM_VEXPRESS_SPC_CPUFREQ
           This add the CPUfreq driver support for Versatile Express
 	  big.LITTLE platforms using SPC for power management.
 
+config ARM_SCPI_CPUFREQ
+        tristate "SCPI based CPUfreq driver"
+	depends on ARM_BIG_LITTLE_CPUFREQ && ARM_SCPI_PROTOCOL
+        help
+	  This adds the CPUfreq driver support for ARM big.LITTLE platforms
+	  using SCPI protocol for CPU power management.
+
+	  This driver uses SCPI Message Protocol driver to interact with the
+	  firmware providing the CPU DVFS functionality.
 
 config ARM_EXYNOS_CPUFREQ
 	tristate "SAMSUNG EXYNOS CPUfreq Driver"
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 2169bf792db7..7e07f5535c3a 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -78,6 +78,7 @@ obj-$(CONFIG_ARM_SA1110_CPUFREQ)	+= sa1110-cpufreq.o
 obj-$(CONFIG_ARM_SPEAR_CPUFREQ)		+= spear-cpufreq.o
 obj-$(CONFIG_ARM_TEGRA_CPUFREQ)		+= tegra-cpufreq.o
 obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ)	+= vexpress-spc-cpufreq.o
+obj-$(CONFIG_ARM_SCPI_CPUFREQ)		+= scpi-cpufreq.o
 
 ##################################################################################
 # PowerPC platform drivers
diff --git a/drivers/cpufreq/scpi-cpufreq.c b/drivers/cpufreq/scpi-cpufreq.c
new file mode 100644
index 000000000000..2c3b16fd3a01
--- /dev/null
+++ b/drivers/cpufreq/scpi-cpufreq.c
@@ -0,0 +1,124 @@
+/*
+ * System Control and Power Interface (SCPI) based CPUFreq Interface driver
+ *
+ * It provides necessary ops to arm_big_little cpufreq driver.
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ * Sudeep Holla <sudeep.holla@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/cpufreq.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/scpi_protocol.h>
+#include <linux/types.h>
+
+#include "arm_big_little.h"
+
+static struct scpi_ops *scpi_ops;
+
+static struct scpi_dvfs_info *scpi_get_dvfs_info(struct device *cpu_dev)
+{
+	u8 domain = topology_physical_package_id(cpu_dev->id);
+
+	if (domain < 0)
+		return ERR_PTR(-EINVAL);
+	return scpi_ops->dvfs_get_info(domain);
+}
+
+static int scpi_opp_table_ops(struct device *cpu_dev, bool remove)
+{
+	int idx, ret = 0;
+	struct scpi_opp *opp;
+	struct scpi_dvfs_info *info = scpi_get_dvfs_info(cpu_dev);
+
+	if (IS_ERR(info))
+		return PTR_ERR(info);
+
+	if (!info->opps)
+		return -EIO;
+
+	for (opp = info->opps, idx = 0; idx < info->count; idx++, opp++) {
+		if (remove)
+			dev_pm_opp_remove(cpu_dev, opp->freq);
+		else
+			ret = dev_pm_opp_add(cpu_dev, opp->freq,
+					     opp->m_volt * 1000);
+		if (ret) {
+			dev_warn(cpu_dev, "failed to add opp %uHz %umV\n",
+				 opp->freq, opp->m_volt);
+			while (idx-- > 0)
+				dev_pm_opp_remove(cpu_dev, (--opp)->freq);
+			return ret;
+		}
+	}
+	return ret;
+}
+
+static int scpi_get_transition_latency(struct device *cpu_dev)
+{
+	struct scpi_dvfs_info *info = scpi_get_dvfs_info(cpu_dev);
+
+	if (IS_ERR(info))
+		return PTR_ERR(info);
+	return info->latency;
+}
+
+static int scpi_init_opp_table(struct device *cpu_dev)
+{
+	return scpi_opp_table_ops(cpu_dev, false);
+}
+
+static void scpi_free_opp_table(struct device *cpu_dev)
+{
+	scpi_opp_table_ops(cpu_dev, true);
+}
+
+static struct cpufreq_arm_bL_ops scpi_cpufreq_ops = {
+	.name	= "scpi",
+	.get_transition_latency = scpi_get_transition_latency,
+	.init_opp_table = scpi_init_opp_table,
+	.free_opp_table = scpi_free_opp_table,
+};
+
+static int scpi_cpufreq_probe(struct platform_device *pdev)
+{
+	scpi_ops = get_scpi_ops();
+	if (!scpi_ops)
+		return -EIO;
+
+	return bL_cpufreq_register(&scpi_cpufreq_ops);
+}
+
+static int scpi_cpufreq_remove(struct platform_device *pdev)
+{
+	bL_cpufreq_unregister(&scpi_cpufreq_ops);
+	scpi_ops = NULL;
+	return 0;
+}
+
+static struct platform_driver scpi_cpufreq_platdrv = {
+	.driver = {
+		.name	= "scpi-cpufreq",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= scpi_cpufreq_probe,
+	.remove		= scpi_cpufreq_remove,
+};
+module_platform_driver(scpi_cpufreq_platdrv);
+
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("ARM SCPI CPUFreq interface driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH v5 6/8] arm64: dts: add SRAM, MHU mailbox and SCPI support on Juno

[Sudeep Holla]

This patch adds support for the MHU mailbox peripheral used on Juno by
application processors to communicate with remote SCP handling most of
the CPU/system power management. It also adds the SRAM reserving the
shared memory and SCPI message protocol using that shared memory.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Jon Medhurst (Tixy) <tixy@linaro.org>
---
 arch/arm64/boot/dts/arm/juno-base.dtsi | 54 ++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index e3ee96036eca..c624208edef6 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -17,6 +17,18 @@
 		};
 	};
 
+	mailbox: mhu@2b1f0000 {
+		compatible = "arm,mhu", "arm,primecell";
+		reg = <0x0 0x2b1f0000 0x0 0x1000>;
+		interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "mhu_lpri_rx",
+				  "mhu_hpri_rx";
+		#mbox-cells = <1>;
+		clocks = <&soc_refclk100mhz>;
+		clock-names = "apb_pclk";
+	};
+
 	gic: interrupt-controller@2c010000 {
 		compatible = "arm,gic-400", "arm,cortex-a15-gic";
 		reg = <0x0 0x2c010000 0 0x1000>,
@@ -44,6 +56,48 @@
 			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>;
 	};
 
+	sram: sram@2e000000 {
+		compatible = "arm,juno-sram-ns", "mmio-sram";
+		reg = <0x0 0x2e000000 0x0 0x8000>;
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x0 0x2e000000 0x8000>;
+
+		cpu_scp_lpri: scp-shmem@0 {
+			compatible = "arm,juno-scp-shmem";
+			reg = <0x0 0x200>;
+		};
+
+		cpu_scp_hpri: scp-shmem@200 {
+			compatible = "arm,juno-scp-shmem";
+			reg = <0x200 0x200>;
+		};
+	};
+
+	scpi {
+		compatible = "arm,scpi";
+		mboxes = <&mailbox 1>;
+		shmem = <&cpu_scp_hpri>;
+
+		clocks {
+			compatible = "arm,scpi-clocks";
+
+			scpi_dvfs: scpi_clocks@0 {
+				compatible = "arm,scpi-dvfs-clocks";
+				#clock-cells = <1>;
+				clock-indices = <0>, <1>, <2>;
+				clock-output-names = "atlclk", "aplclk","gpuclk";
+			};
+			scpi_clk: scpi_clocks@3 {
+				compatible = "arm,scpi-variable-clocks";
+				#clock-cells = <1>;
+				clock-indices = <3>, <4>;
+				clock-output-names = "pxlclk0", "pxlclk1";
+			};
+		};
+	};
+
 	/include/ "juno-clocks.dtsi"
 
 	dma@7ff00000 {
-- 
1.9.1

[PATCH v5 7/8] arm64: dts: add CPU topology on Juno

[Sudeep Holla]

This patch adds CPU topology on Juno. It will be useful for ther other
IP blocks depending on this topology.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Jon Medhurst (Tixy) <tixy@linaro.org>
---
 arch/arm64/boot/dts/arm/juno-r1.dts | 26 ++++++++++++++++++++++++++
 arch/arm64/boot/dts/arm/juno.dts    | 26 ++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/arch/arm64/boot/dts/arm/juno-r1.dts b/arch/arm64/boot/dts/arm/juno-r1.dts
index c62751153a4f..69130840c6cd 100644
--- a/arch/arm64/boot/dts/arm/juno-r1.dts
+++ b/arch/arm64/boot/dts/arm/juno-r1.dts
@@ -34,6 +34,32 @@
 		#address-cells = <2>;
 		#size-cells = <0>;
 
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&A57_0>;
+				};
+				core1 {
+					cpu = <&A57_1>;
+				};
+			};
+
+			cluster1 {
+				core0 {
+					cpu = <&A53_0>;
+				};
+				core1 {
+					cpu = <&A53_1>;
+				};
+				core2 {
+					cpu = <&A53_2>;
+				};
+				core3 {
+					cpu = <&A53_3>;
+				};
+			};
+		};
+
 		A57_0: cpu@0 {
 			compatible = "arm,cortex-a57","arm,armv8";
 			reg = <0x0 0x0>;
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index d7cbdd482a61..ce1128a54c8d 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -34,6 +34,32 @@
 		#address-cells = <2>;
 		#size-cells = <0>;
 
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&A57_0>;
+				};
+				core1 {
+					cpu = <&A57_1>;
+				};
+			};
+
+			cluster1 {
+				core0 {
+					cpu = <&A53_0>;
+				};
+				core1 {
+					cpu = <&A53_1>;
+				};
+				core2 {
+					cpu = <&A53_2>;
+				};
+				core3 {
+					cpu = <&A53_3>;
+				};
+			};
+		};
+
 		A57_0: cpu@0 {
 			compatible = "arm,cortex-a57","arm,armv8";
 			reg = <0x0 0x0>;
-- 
1.9.1

[PATCH v5 8/8] arm64: dts: add clock support for all the cpus

[Sudeep Holla]

This patch adds the CPU clocks so that the CPU DVFS can be enabled.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Jon Medhurst (Tixy) <tixy@linaro.org>
---
 arch/arm64/boot/dts/arm/juno-r1.dts | 6 ++++++
 arch/arm64/boot/dts/arm/juno.dts    | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/arm/juno-r1.dts b/arch/arm64/boot/dts/arm/juno-r1.dts
index 69130840c6cd..5eef4aa0c532 100644
--- a/arch/arm64/boot/dts/arm/juno-r1.dts
+++ b/arch/arm64/boot/dts/arm/juno-r1.dts
@@ -66,6 +66,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A57_L2>;
+			clocks = <&scpi_dvfs 0>;
 		};
 
 		A57_1: cpu@1 {
@@ -74,6 +75,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A57_L2>;
+			clocks = <&scpi_dvfs 0>;
 		};
 
 		A53_0: cpu@100 {
@@ -82,6 +84,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			clocks = <&scpi_dvfs 1>;
 		};
 
 		A53_1: cpu@101 {
@@ -90,6 +93,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			clocks = <&scpi_dvfs 1>;
 		};
 
 		A53_2: cpu@102 {
@@ -98,6 +102,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			clocks = <&scpi_dvfs 1>;
 		};
 
 		A53_3: cpu@103 {
@@ -106,6 +111,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			clocks = <&scpi_dvfs 1>;
 		};
 
 		A57_L2: l2-cache0 {
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index ce1128a54c8d..c02f880584e8 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -66,6 +66,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A57_L2>;
+			clocks = <&scpi_dvfs 0>;
 		};
 
 		A57_1: cpu@1 {
@@ -74,6 +75,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A57_L2>;
+			clocks = <&scpi_dvfs 0>;
 		};
 
 		A53_0: cpu@100 {
@@ -82,6 +84,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			clocks = <&scpi_dvfs 1>;
 		};
 
 		A53_1: cpu@101 {
@@ -90,6 +93,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			clocks = <&scpi_dvfs 1>;
 		};
 
 		A53_2: cpu@102 {
@@ -98,6 +102,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			clocks = <&scpi_dvfs 1>;
 		};
 
 		A53_3: cpu@103 {
@@ -106,6 +111,7 @@
 			device_type = "cpu";
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			clocks = <&scpi_dvfs 1>;
 		};
 
 		A57_L2: l2-cache0 {
-- 
1.9.1

Re: [PATCH v5 5/8] cpufreq: arm_big_little: add SCPI interface driver

[Sudeep Holla]

On 23/07/15 12:10, Sudeep Holla wrote:
> On some ARM based systems, a separate Cortex-M based System Control
> Processor(SCP) provides the overall power, clock, reset and system
> control including CPU DVFS. SCPI Message Protocol is used to
> communicate with the SCPI.
>
> This patch adds a interface driver for adding OPPs and registering
> the arm_big_little cpufreq driver for such systems.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: linux-pm@vger.kernel.org
> ---
>   MAINTAINERS                    |   1 +
>   drivers/cpufreq/Kconfig.arm    |   9 +++
>   drivers/cpufreq/Makefile       |   1 +
>   drivers/cpufreq/scpi-cpufreq.c | 124 +++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 135 insertions(+)
>   create mode 100644 drivers/cpufreq/scpi-cpufreq.c
>
> Hi Rafael,
>
> Viresh has already ACKed, since I plan to push this as a series via arm-soc,
> can you provide your ACK if the patch looks fine to you ?
>

Any objections to take it via ARM-SoC ? If not can you please provide ACK ?

Regards,
Sudeep

Re: [PATCH v5 3/8] clk: add support for clocks provided by SCP(System Control Processor)

[Sudeep Holla]

Hi Stephen/Mike,

On 23/07/15 12:10, Sudeep Holla wrote:
> On some ARM based systems, a separate Cortex-M based System Control
> Processor(SCP) provides the overall power, clock, reset and system
> control. System Control and Power Interface(SCPI) Message Protocol
> is defined for the communication between the Application Cores(AP)
> and the SCP.
>
> This patch adds support for the clocks provided by SCP using SCPI
> protocol.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Mike Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@codeaurora.org>

Can one of you provide ACK if this patch looks fine, so that I can take 
this series via ARM-SoC ?

Regards,
Sudeep

Re: [PATCH v5 1/8] Documentation: add DT binding for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

Hi Mark,

On 23/07/15 12:10, Sudeep Holla wrote:
> This patch adds devicetree binding for System Control and Power
> Interface (SCPI) Message Protocol used between the Application Cores(AP)
> and the System Control Processor(SCP). The MHU peripheral provides a
> mechanism for inter-processor communication between SCP's M3 processor
> and AP.
>
> SCP offers control and management of the core/cluster power states,
> various power domain DVFS including the core/cluster, certain system
> clocks configuration, thermal sensors and many others.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>

Can you review or provide ACK if this looks good ?

[..]

> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> new file mode 100644
> index 000000000000..e21cce646561
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> @@ -0,0 +1,151 @@

[..]

> +cpu@0 {
> +	...
> +	reg = <0 0>;
> +	clocks = <&scpi_dvfs 0>;
> +};
> +
> +hdlcd@7ff60000 {
> +	...
> +	reg = <0 0x7ff60000 0 0x1000>;
> +	clocks = <&scpi_clk 4>;
> +};
> +
> +In the above example, the #clock-cells is set to 1 as required.
> +scpi_dvfs has 3 output clocks namely: vbig, vlittle and vgpu with 0, 1

I noticed this just after sending it out, fixed now to match the names
in the example correctly.

Regards,
Sudeep

Re: [PATCH v5 1/8] Documentation: add DT binding for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

Hi Mark,

On 23/07/15 12:10, Sudeep Holla wrote:
> This patch adds devicetree binding for System Control and Power
> Interface (SCPI) Message Protocol used between the Application Cores(AP)
> and the System Control Processor(SCP). The MHU peripheral provides a
> mechanism for inter-processor communication between SCP's M3 processor
> and AP.
>
> SCP offers control and management of the core/cluster power states,
> various power domain DVFS including the core/cluster, certain system
> clocks configuration, thermal sensors and many others.
>
> Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>

Can you review or provide ACK if this looks good ?

[..]

> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> new file mode 100644
> index 000000000000..e21cce646561
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> @@ -0,0 +1,151 @@

[..]

> +cpu@0 {
> +	...
> +	reg = <0 0>;
> +	clocks = <&scpi_dvfs 0>;
> +};
> +
> +hdlcd@7ff60000 {
> +	...
> +	reg = <0 0x7ff60000 0 0x1000>;
> +	clocks = <&scpi_clk 4>;
> +};
> +
> +In the above example, the #clock-cells is set to 1 as required.
> +scpi_dvfs has 3 output clocks namely: vbig, vlittle and vgpu with 0, 1

I noticed this just after sending it out, fixed now to match the names
in the example correctly.

Regards,
Sudeep
--
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

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Thu, Jul 23, 2015 at 4:40 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:

...

> +static int scpi_probe(struct platform_device *pdev)
> +{
> +       int count, idx, ret;
> +       struct resource res;
> +       struct scpi_chan *scpi_chan;
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = dev->of_node;
> +
> +       scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
> +       if (!scpi_info)
> +               return -ENOMEM;
> +
> +       count = of_count_phandle_with_args(np, "mboxes", "#mbox-cells");
> +       if (count < 0) {
> +               dev_err(dev, "no mboxes property in '%s'\n", np->full_name);
> +               return -ENODEV;
> +       }
> +
> +       scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan), GFP_KERNEL);
> +       if (!scpi_chan)
> +               return -ENOMEM;
> +
> +       for (idx = 0; idx < count; idx++) {
> +               resource_size_t size;
> +               struct scpi_chan *pchan = scpi_chan + idx;
> +               struct mbox_client *cl = &pchan->cl;
> +               struct device_node *shmem = of_parse_phandle(np, "shmem", idx);
> +
> +               if (of_address_to_resource(shmem, 0, &res)) {
> +                       dev_err(dev, "failed to get SCPI payload mem resource\n");
> +                       ret = -EINVAL;
> +                       goto err;
> +               }
> +
> +               size = resource_size(&res);
> +               pchan->rx_payload = devm_ioremap(dev, res.start, size);
> +               if (!pchan->rx_payload) {
> +                       dev_err(dev, "failed to ioremap SCPI payload\n");
> +                       ret = -EADDRNOTAVAIL;
> +                       goto err;
> +               }
> +               pchan->tx_payload = pchan->rx_payload + (size >> 1);
> +
> +               cl->dev = dev;
> +               cl->rx_callback = scpi_handle_remote_msg;
> +               cl->tx_prepare = scpi_tx_prepare;
> +               cl->tx_block = true;
> +               cl->tx_tout = 50;
> +               cl->knows_txdone = false; /* controller can ack */
>
 This is the cause of your problems that you think should be solved by
using hrtimer.

 Controller may or may not (like MHU) set txdone_irq. However every
scpi command (struct scpi_ops members) is replied to as a response
packet reporting success or failure.
So the client should set 'knows_txdone' to be true unless it is told
the controller on that platform supports txdone_irq (what you call
'ack').

-Jassi

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 29/07/15 09:05, Jassi Brar wrote:
> On Thu, Jul 23, 2015 at 4:40 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> ...
>
>> +static int scpi_probe(struct platform_device *pdev)
>> +{
>> +       int count, idx, ret;
>> +       struct resource res;
>> +       struct scpi_chan *scpi_chan;
>> +       struct device *dev = &pdev->dev;
>> +       struct device_node *np = dev->of_node;
>> +
>> +       scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
>> +       if (!scpi_info)
>> +               return -ENOMEM;
>> +
>> +       count = of_count_phandle_with_args(np, "mboxes", "#mbox-cells");
>> +       if (count < 0) {
>> +               dev_err(dev, "no mboxes property in '%s'\n", np->full_name);
>> +               return -ENODEV;
>> +       }
>> +
>> +       scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan), GFP_KERNEL);
>> +       if (!scpi_chan)
>> +               return -ENOMEM;
>> +
>> +       for (idx = 0; idx < count; idx++) {
>> +               resource_size_t size;
>> +               struct scpi_chan *pchan = scpi_chan + idx;
>> +               struct mbox_client *cl = &pchan->cl;
>> +               struct device_node *shmem = of_parse_phandle(np, "shmem", idx);
>> +
>> +               if (of_address_to_resource(shmem, 0, &res)) {
>> +                       dev_err(dev, "failed to get SCPI payload mem resource\n");
>> +                       ret = -EINVAL;
>> +                       goto err;
>> +               }
>> +
>> +               size = resource_size(&res);
>> +               pchan->rx_payload = devm_ioremap(dev, res.start, size);
>> +               if (!pchan->rx_payload) {
>> +                       dev_err(dev, "failed to ioremap SCPI payload\n");
>> +                       ret = -EADDRNOTAVAIL;
>> +                       goto err;
>> +               }
>> +               pchan->tx_payload = pchan->rx_payload + (size >> 1);
>> +
>> +               cl->dev = dev;
>> +               cl->rx_callback = scpi_handle_remote_msg;
>> +               cl->tx_prepare = scpi_tx_prepare;
>> +               cl->tx_block = true;
>> +               cl->tx_tout = 50;
>> +               cl->knows_txdone = false; /* controller can ack */
>>
>   This is the cause of your problems that you think should be solved by
> using hrtimer.
>

Ah sorry, it's stupid mistake on my part while writing the comment. It
should have been controller can't ack, fixed locally now thanks for
pointing it out.

>   Controller may or may not (like MHU) set txdone_irq. However every
> scpi command (struct scpi_ops members) is replied to as a response
> packet reporting success or failure.

No that's not true, I have already mentioned that couple of times in the
other thread. It's just wrong comment here which went unnoticed from
day#1, sorry for that.

> So the client should set 'knows_txdone' to be true unless it is told
> the controller on that platform supports txdone_irq (what you call
> 'ack').
>

I got the concept but SCP can't ack via protocol, protocol has no such
provision and it sets flags in MHU status register.

Regards,
Sudeep

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Wed, Jul 29, 2015 at 2:08 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> On 29/07/15 09:05, Jassi Brar wrote:
>
>>
>>> +static int scpi_probe(struct platform_device *pdev)
>>> +{
>>> +       int count, idx, ret;
>>> +       struct resource res;
>>> +       struct scpi_chan *scpi_chan;
>>> +       struct device *dev = &pdev->dev;
>>> +       struct device_node *np = dev->of_node;
>>> +
>>> +       scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
>>> +       if (!scpi_info)
>>> +               return -ENOMEM;
>>> +
>>> +       count = of_count_phandle_with_args(np, "mboxes", "#mbox-cells");
>>> +       if (count < 0) {
>>> +               dev_err(dev, "no mboxes property in '%s'\n",
>>> np->full_name);
>>> +               return -ENODEV;
>>> +       }
>>> +
>>> +       scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan),
>>> GFP_KERNEL);
>>> +       if (!scpi_chan)
>>> +               return -ENOMEM;
>>> +
>>> +       for (idx = 0; idx < count; idx++) {
>>> +               resource_size_t size;
>>> +               struct scpi_chan *pchan = scpi_chan + idx;
>>> +               struct mbox_client *cl = &pchan->cl;
>>> +               struct device_node *shmem = of_parse_phandle(np, "shmem",
>>> idx);
>>> +
>>> +               if (of_address_to_resource(shmem, 0, &res)) {
>>> +                       dev_err(dev, "failed to get SCPI payload mem
>>> resource\n");
>>> +                       ret = -EINVAL;
>>> +                       goto err;
>>> +               }
>>> +
>>> +               size = resource_size(&res);
>>> +               pchan->rx_payload = devm_ioremap(dev, res.start, size);
>>> +               if (!pchan->rx_payload) {
>>> +                       dev_err(dev, "failed to ioremap SCPI payload\n");
>>> +                       ret = -EADDRNOTAVAIL;
>>> +                       goto err;
>>> +               }
>>> +               pchan->tx_payload = pchan->rx_payload + (size >> 1);
>>> +
>>> +               cl->dev = dev;
>>> +               cl->rx_callback = scpi_handle_remote_msg;
>>> +               cl->tx_prepare = scpi_tx_prepare;
>>> +               cl->tx_block = true;
>>> +               cl->tx_tout = 50;
>>> +               cl->knows_txdone = false; /* controller can ack */
>>>
>>   This is the cause of your problems that you think should be solved by
>> using hrtimer.
>>
>
> Ah sorry, it's stupid mistake on my part while writing the comment. It
> should have been controller can't ack, fixed locally now thanks for
> pointing it out.
>
No, I am talking about code, not the comment.

>>   Controller may or may not (like MHU) set txdone_irq. However every
>> scpi command (struct scpi_ops members) is replied to as a response
>> packet reporting success or failure.
>
>
> No that's not true, I have already mentioned that couple of times in the
> other thread. It's just wrong comment here which went unnoticed from
> day#1, sorry for that.
>
>> So the client should set 'knows_txdone' to be true unless it is told
>> the controller on that platform supports txdone_irq (what you call
>> 'ack').
>>
> I got the concept but SCP can't ack via protocol, protocol has no such
> provision and it sets flags in MHU status register.
>
You either don't get the concept of TXDONE_BY_ACK  or deliberately
overlook my point.

Assuming the former, let me explain. When a client receives a
response, it can be sure that the request has already been read by the
remote. If the protocol specifies every request has some response, the
client should assert 'knows_txdone' and call mbox_client_txdone() upon
receiving a reply packet.
   So I said,  cl->knows_txdone = false;   is the root of problems you
report. If you fix that, the performance should be even better than
using hrtimer.

-Jassi

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 29/07/15 12:19, Jassi Brar wrote:
> On Wed, Jul 29, 2015 at 2:08 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> On 29/07/15 09:05, Jassi Brar wrote:
>>
>>>
>>>> +static int scpi_probe(struct platform_device *pdev)
>>>> +{
>>>> +       int count, idx, ret;
>>>> +       struct resource res;
>>>> +       struct scpi_chan *scpi_chan;
>>>> +       struct device *dev = &pdev->dev;
>>>> +       struct device_node *np = dev->of_node;
>>>> +
>>>> +       scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
>>>> +       if (!scpi_info)
>>>> +               return -ENOMEM;
>>>> +
>>>> +       count = of_count_phandle_with_args(np, "mboxes", "#mbox-cells");
>>>> +       if (count < 0) {
>>>> +               dev_err(dev, "no mboxes property in '%s'\n",
>>>> np->full_name);
>>>> +               return -ENODEV;
>>>> +       }
>>>> +
>>>> +       scpi_chan = devm_kcalloc(dev, count, sizeof(*scpi_chan),
>>>> GFP_KERNEL);
>>>> +       if (!scpi_chan)
>>>> +               return -ENOMEM;
>>>> +
>>>> +       for (idx = 0; idx < count; idx++) {
>>>> +               resource_size_t size;
>>>> +               struct scpi_chan *pchan = scpi_chan + idx;
>>>> +               struct mbox_client *cl = &pchan->cl;
>>>> +               struct device_node *shmem = of_parse_phandle(np, "shmem",
>>>> idx);
>>>> +
>>>> +               if (of_address_to_resource(shmem, 0, &res)) {
>>>> +                       dev_err(dev, "failed to get SCPI payload mem
>>>> resource\n");
>>>> +                       ret = -EINVAL;
>>>> +                       goto err;
>>>> +               }
>>>> +
>>>> +               size = resource_size(&res);
>>>> +               pchan->rx_payload = devm_ioremap(dev, res.start, size);
>>>> +               if (!pchan->rx_payload) {
>>>> +                       dev_err(dev, "failed to ioremap SCPI payload\n");
>>>> +                       ret = -EADDRNOTAVAIL;
>>>> +                       goto err;
>>>> +               }
>>>> +               pchan->tx_payload = pchan->rx_payload + (size >> 1);
>>>> +
>>>> +               cl->dev = dev;
>>>> +               cl->rx_callback = scpi_handle_remote_msg;
>>>> +               cl->tx_prepare = scpi_tx_prepare;
>>>> +               cl->tx_block = true;
>>>> +               cl->tx_tout = 50;
>>>> +               cl->knows_txdone = false; /* controller can ack */
>>>>
>>>    This is the cause of your problems that you think should be solved by
>>> using hrtimer.
>>>
>>
>> Ah sorry, it's stupid mistake on my part while writing the comment. It
>> should have been controller can't ack, fixed locally now thanks for
>> pointing it out.
>>
> No, I am talking about code, not the comment.
>
>>>    Controller may or may not (like MHU) set txdone_irq. However every
>>> scpi command (struct scpi_ops members) is replied to as a response
>>> packet reporting success or failure.
>>
>>
>> No that's not true, I have already mentioned that couple of times in the
>> other thread. It's just wrong comment here which went unnoticed from
>> day#1, sorry for that.
>>
>>> So the client should set 'knows_txdone' to be true unless it is told
>>> the controller on that platform supports txdone_irq (what you call
>>> 'ack').
>>>
>> I got the concept but SCP can't ack via protocol, protocol has no such
>> provision and it sets flags in MHU status register.
>>
> You either don't get the concept of TXDONE_BY_ACK  or deliberately
> overlook my point.
>

No I do understand the concept and didn't overlook the points you made.

> Assuming the former, let me explain. When a client receives a
> response, it can be sure that the request has already been read by the
> remote.

Waiting for the response would be too late for few expensive commands
(e.g setting up external regulators). The remote firmware acknowledges
Tx by setting status flags and will be ready to accept new commands.

> If the protocol specifies every request has some response, the

Not always true there can be few commands without response. The protocol
specifies that we need check the status flag before sending the new
command as it's bidirectional, hence polling is recommended (Section 2.2
Communication flow in the SCPI specification)

> client should assert 'knows_txdone' and call mbox_client_txdone() upon
> receiving a reply packet.

Since this is not always true and not recommended in the specification,
I am hesitant to use this option as the firmware can always change their
internal mechanics without breaking the protocol. We need to ensure we 
are compliant to the spec.

> So I said,  cl->knows_txdone = false;   is the root of problems you

It could be and won't rule that out. I would prefer using knows_txdone
and use mbox_client_txdone if feasible, but I can't as the without
violating the specification.

FYI, I had tried it and ended up with issues in the firmware. The
argument from the firmware is that we aren't specification compliant,
so I had to use polling.

> report. If you fix that, the performance should be even better than
> using hrtimer.
>

That would have been ideal and much better but I can't use that for
above mentioned reason.

Though you had valid concerns here and I hope I clarified them all
sucessfully, none were related to hrtimers.

Can I assume you are fine with hrtimers ? If so, can you review this patch ?

Regards,
Sudeep

Re: [PATCH v5 3/8] clk: add support for clocks provided by SCP(System Control Processor)

[Stephen Boyd]

On 07/23/2015 04:10 AM, Sudeep Holla wrote:
> On some ARM based systems, a separate Cortex-M based System Control
> Processor(SCP) provides the overall power, clock, reset and system
> control. System Control and Power Interface(SCPI) Message Protocol
> is defined for the communication between the Application Cores(AP)
> and the SCP.
>
> This patch adds support for the clocks provided by SCP using SCPI
> protocol.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Mike Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Jon Medhurst (Tixy) <tixy@linaro.org>
> Cc: linux-clk@vger.kernel.org

Looks good to me, modulo one comment.

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

> +
> +static const struct of_device_id scpi_clocks_ids[] = {
> +	{ .compatible = "arm,scpi-clocks", },
> +	{}
> +};

Did you want a MODULE_DEVICE_TABLE() here?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v5 3/8] clk: add support for clocks provided by SCP(System Control Processor)

[Sudeep Holla]

On 29/07/15 18:37, Stephen Boyd wrote:
> On 07/23/2015 04:10 AM, Sudeep Holla wrote:
>> On some ARM based systems, a separate Cortex-M based System Control
>> Processor(SCP) provides the overall power, clock, reset and system
>> control. System Control and Power Interface(SCPI) Message Protocol
>> is defined for the communication between the Application Cores(AP)
>> and the SCP.
>>
>> This patch adds support for the clocks provided by SCP using SCPI
>> protocol.
>>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Mike Turquette <mturquette@baylibre.com>
>> Cc: Stephen Boyd <sboyd@codeaurora.org>
>> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Jon Medhurst (Tixy) <tixy@linaro.org>
>> Cc: linux-clk@vger.kernel.org
>
> Looks good to me, modulo one comment.
>
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
>

Thanks, I assume you are fine if this gets merged via arm-soc

>> +
>> +static const struct of_device_id scpi_clocks_ids[] = {
>> +	{ .compatible = "arm,scpi-clocks", },
>> +	{}
>> +};
>
> Did you want a MODULE_DEVICE_TABLE() here?
>

Yes, I do and it's added now.

Regards,
Sudeep

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Wed, Jul 29, 2015 at 6:20 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> On 29/07/15 12:19, Jassi Brar wrote:
>
>> Assuming the former, let me explain. When a client receives a
>> response, it can be sure that the request has already been read by the
>> remote.
>
> Waiting for the response would be too late for few expensive commands
> (e.g setting up external regulators). The remote firmware acknowledges
> Tx by setting status flags and will be ready to accept new commands.
>
No. Polling still happens. If anything, mbox_client_txdone() should
only speed up things.

>> If the protocol specifies every request has some response, the
>
> Not always true there can be few commands without response. The protocol
> specifies that we need check the status flag before sending the new
> command as it's bidirectional, hence polling is recommended (Section 2.2
> Communication flow in the SCPI specification)
>
mbox_client_txdone() will only be called for commands that has some
response. Commands that don't have a response would be completed by
polling.

>> client should assert 'knows_txdone' and call mbox_client_txdone() upon
>> receiving a reply packet.
>
> Since this is not always true and not recommended in the specification,
> I am hesitant to use this option as the firmware can always change their
> internal mechanics without breaking the protocol. We need to ensure we are
> compliant to the spec.
>
I don't see how it could break compliance.

>> So I said,  cl->knows_txdone = false;   is the root of problems you
>
> It could be and won't rule that out. I would prefer using knows_txdone
> and use mbox_client_txdone if feasible, but I can't as the without
> violating the specification.
>
> FYI, I had tried it and ended up with issues in the firmware. The
> argument from the firmware is that we aren't specification compliant,
> so I had to use polling.
>
I am sure you would have copy of that discarded code. Care to share? I
can't imagine how we handle completions locally could affect the
remote. The mbox_client_txdone() is untested so I don't rule out bugs,
otherwise it should only make things better.

Jassi

Re: [PATCH v5 3/8] clk: add support for clocks provided by SCP(System Control Processor)

[Stephen Boyd]

On 07/30, Sudeep Holla wrote:
> 
> 
> On 29/07/15 18:37, Stephen Boyd wrote:
> >On 07/23/2015 04:10 AM, Sudeep Holla wrote:
> >>On some ARM based systems, a separate Cortex-M based System Control
> >>Processor(SCP) provides the overall power, clock, reset and system
> >>control. System Control and Power Interface(SCPI) Message Protocol
> >>is defined for the communication between the Application Cores(AP)
> >>and the SCP.
> >>
> >>This patch adds support for the clocks provided by SCP using SCPI
> >>protocol.
> >>
> >>Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >>Cc: Mike Turquette <mturquette@baylibre.com>
> >>Cc: Stephen Boyd <sboyd@codeaurora.org>
> >>Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> >>Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >>Cc: Jon Medhurst (Tixy) <tixy@linaro.org>
> >>Cc: linux-clk@vger.kernel.org
> >
> >Looks good to me, modulo one comment.
> >
> >Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
> >
> 
> Thanks, I assume you are fine if this gets merged via arm-soc

Yes that's fine.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 30/07/15 18:56, Jassi Brar wrote:
> On Wed, Jul 29, 2015 at 6:20 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> On 29/07/15 12:19, Jassi Brar wrote:
>>
>>> Assuming the former, let me explain. When a client receives a
>>> response, it can be sure that the request has already been read by the
>>> remote.
>>
>> Waiting for the response would be too late for few expensive commands
>> (e.g setting up external regulators). The remote firmware acknowledges
>> Tx by setting status flags and will be ready to accept new commands.
>>
> No. Polling still happens. If anything, mbox_client_txdone() should
> only speed up things.
>

Yes I understand and that's good.

>>> If the protocol specifies every request has some response, the
>>
>> Not always true there can be few commands without response. The protocol
>> specifies that we need check the status flag before sending the new
>> command as it's bidirectional, hence polling is recommended (Section 2.2
>> Communication flow in the SCPI specification)
>>
> mbox_client_txdone() will only be called for commands that has some
> response. Commands that don't have a response would be completed by
> polling.
>

OK, got it

>>> client should assert 'knows_txdone' and call mbox_client_txdone() upon
>>> receiving a reply packet.
>>
>> Since this is not always true and not recommended in the specification,
>> I am hesitant to use this option as the firmware can always change their
>> internal mechanics without breaking the protocol. We need to ensure we are
>> compliant to the spec.
>>
> I don't see how it could break compliance.
>

While I agree it shouldn't, the firmware guys won't support if we
deviate from the spec. I won't get support for firmware bug fixes in
that case.

Having said that, I don't rule out the usage of TX_BY_ACK, I will need
more time for testing(usually we stress test firmware using Linux for
few of days continuously as we have hit issues after that :)) and
getting things fixed if anything breaks.

>>> So I said,  cl->knows_txdone = false;   is the root of problems you
>>
>> It could be and won't rule that out. I would prefer using knows_txdone
>> and use mbox_client_txdone if feasible, but I can't as the without
>> violating the specification.
>>
>> FYI, I had tried it and ended up with issues in the firmware. The
>> argument from the firmware is that we aren't specification compliant,
>> so I had to use polling.
>>
> I am sure you would have copy of that discarded code. Care to share? I
> can't imagine how we handle completions locally could affect the
> remote. The mbox_client_txdone() is untested so I don't rule out bugs,
> otherwise it should only make things better.
>

I tested it with very old firmware almost 4-5 months back. I don't have
the patch on top of this series handy, but will dig it out and give it a
try with latest firmware. I will let you know the results.

For now, I would keep this just polling and unblock others who are
waiting on this series.

Regards,
Sudeep

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 30/07/15 18:56, Jassi Brar wrote:
> On Wed, Jul 29, 2015 at 6:20 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> On 29/07/15 12:19, Jassi Brar wrote:
>>
>>> Assuming the former, let me explain. When a client receives a
>>> response, it can be sure that the request has already been read by the
>>> remote.
>>
>> Waiting for the response would be too late for few expensive commands
>> (e.g setting up external regulators). The remote firmware acknowledges
>> Tx by setting status flags and will be ready to accept new commands.
>>
> No. Polling still happens. If anything, mbox_client_txdone() should
> only speed up things.
>
>>> If the protocol specifies every request has some response, the
>>
>> Not always true there can be few commands without response. The protocol
>> specifies that we need check the status flag before sending the new
>> command as it's bidirectional, hence polling is recommended (Section 2.2
>> Communication flow in the SCPI specification)
>>
> mbox_client_txdone() will only be called for commands that has some
> response. Commands that don't have a response would be completed by
> polling.
>

I forgot to mention, we have a the following description in
mbox_client_txdone which is misleading:

"The client/protocol had received some 'ACK' packet and it notifies the
API that the last packet was sent successfully. This only works if the
controller can't sense TX-Done."

which is clearly not the case in SCPI. IMO we may have to reword that.

Regards,
Sudeep

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> I forgot to mention, we have a the following description in
> mbox_client_txdone which is misleading:
>
> "The client/protocol had received some 'ACK' packet and it notifies the
> API that the last packet was sent successfully. This only works if the
> controller can't sense TX-Done."
>
> which is clearly not the case in SCPI. IMO we may have to reword that.
>
Yes. And also see whether it could race with polling driven tx_tick.

Thanks.

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 31/07/15 11:38, Jassi Brar wrote:
> On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>> I forgot to mention, we have a the following description in
>> mbox_client_txdone which is misleading:
>>
>> "The client/protocol had received some 'ACK' packet and it notifies the
>> API that the last packet was sent successfully. This only works if the
>> controller can't sense TX-Done."
>>
>> which is clearly not the case in SCPI. IMO we may have to reword that.
>>
> Yes. And also see whether it could race with polling driven tx_tick.
>

Yes I am also looking at that now while I am trying to check if
TXDONE_BY_ACK works on Juno, will keep you posted.

Regards,
Sudeep

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 31/07/15 11:43, Sudeep Holla wrote:
>
>
> On 31/07/15 11:38, Jassi Brar wrote:
>> On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>
>>> I forgot to mention, we have a the following description in
>>> mbox_client_txdone which is misleading:
>>>
>>> "The client/protocol had received some 'ACK' packet and it notifies the
>>> API that the last packet was sent successfully. This only works if the
>>> controller can't sense TX-Done."
>>>
>>> which is clearly not the case in SCPI. IMO we may have to reword that.
>>>
>> Yes. And also see whether it could race with polling driven tx_tick.
>>
>
> Yes I am also looking at that now while I am trying to check if
> TXDONE_BY_ACK works on Juno, will keep you posted.
>

OK, I recollect the racy condition now which I had in my mind from the
beginning convincing myself why we can't use that option. I was not good
in words to explain it so far but let me try with the ASCII art this
time. Note Tx ACK below means the remote setting the register flag and
not to be confused with the ACK packet. For simplicity Rx can be assumed
to be Tx ACK packet

      Time       MHU/SCPI                        Remote SCP
                    |                                |
         1          |------------ Tx1 -------------->|
                    |                                |
         2          |<----------- Tx1 ACK -----------|
                    |                                |
         3          |------------ Tx2 -------------->|
                    |                                |
         4          |<----------- Rx1 ---------------|
                    |                                |
         5          |<----------- Tx2 ACK -----------|
                    |                                |
         6          |------------ Tx3 -------------->|
                    |                                |
         7          |<----------- Rx2 ---------------|

Now lets consider the above scenario, suppose we have TXDONE_BY_ACK
and use mbox_client_txdone in Rx interrupt(i.e. response packet), we end
up in the race easily IIUC.

E.g. A client would have sent Tx2(3) before Rx1 interrupt(4) and if we
ACK Tx1 now in (3), we will end up acknowledging Tx2(5) as the mailbox
core assumes only one Tx request at a time which is clearly not the case
in our setup. The client can then go ahead and send Tx3(6) overwriting
the payload while remote was processing or even result in remote missing
the request completely. Does that make sense or am I missing something ?

Regards,
Sudeep

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Jassi Brar]

On Fri, Jul 31, 2015 at 6:38 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 31/07/15 11:43, Sudeep Holla wrote:
>>
>>
>>
>> On 31/07/15 11:38, Jassi Brar wrote:
>>>
>>> On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla <sudeep.holla@arm.com>
>>> wrote:
>>>>
>>>>
>>>> I forgot to mention, we have a the following description in
>>>> mbox_client_txdone which is misleading:
>>>>
>>>> "The client/protocol had received some 'ACK' packet and it notifies the
>>>> API that the last packet was sent successfully. This only works if the
>>>> controller can't sense TX-Done."
>>>>
>>>> which is clearly not the case in SCPI. IMO we may have to reword that.
>>>>
>>> Yes. And also see whether it could race with polling driven tx_tick.
>>>
>>
>> Yes I am also looking at that now while I am trying to check if
>> TXDONE_BY_ACK works on Juno, will keep you posted.
>>
>
> OK, I recollect the racy condition now which I had in my mind from the
> beginning convincing myself why we can't use that option. I was not good
> in words to explain it so far but let me try with the ASCII art this
> time. Note Tx ACK below means the remote setting the register flag and
> not to be confused with the ACK packet. For simplicity Rx can be assumed
> to be Tx ACK packet
>
>      Time       MHU/SCPI                        Remote SCP
>                    |                                |
>         1          |------------ Tx1 -------------->|
>                    |                                |
>         2          |<----------- Tx1 ACK -----------|
>                    |                                |
>         3          |------------ Tx2 -------------->|
>                    |                                |
>         4          |<----------- Rx1 ---------------|
>                    |                                |
>         5          |<----------- Tx2 ACK -----------|
>                    |                                |
>         6          |------------ Tx3 -------------->|
>                    |                                |
>         7          |<----------- Rx2 ---------------|
>
> Now lets consider the above scenario, suppose we have TXDONE_BY_ACK
> and use mbox_client_txdone in Rx interrupt(i.e. response packet), we end
> up in the race easily IIUC.
>
> E.g. A client would have sent Tx2(3) before Rx1 interrupt(4) and if we
> ACK Tx1 now in (3), we will end up acknowledging Tx2(5) as the mailbox
> core assumes only one Tx request at a time which is clearly not the case
> in our setup. The client can then go ahead and send Tx3(6) overwriting
> the payload while remote was processing or even result in remote missing
> the request completely. Does that make sense or am I missing something ?
>
Yeah, that could happen. But the race can be fixed by checking
last_tx_done if the controller provides that. If last_tx_done is not
implemented, polling won't race.

Please try the following...

==================>8=================
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 07fd507..c973c2c 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -181,17 +181,23 @@ EXPORT_SYMBOL_GPL(mbox_chan_txdone);
  * @r: Success status of last transmission.
  *
  * The client/protocol had received some 'ACK' packet and it notifies
- * the API that the last packet was sent successfully. This only works
- * if the controller can't sense TX-Done.
+ * the API that the last packet was sent successfully.
  */
 void mbox_client_txdone(struct mbox_chan *chan, int r)
 {
+       bool txdone = true;
+
        if (unlikely(!(chan->txdone_method & TXDONE_BY_ACK))) {
                dev_err(chan->mbox->dev, "Client can't run the TX ticker\n");
                return;
        }

-       tx_tick(chan, r);
+       /* if h/w can check for tx-done, make sure its done */
+       if (chan->mbox->ops->last_tx_done)
+               txdone = chan->mbox->ops->last_tx_done(chan);
+
+       if (txdone)
+               tx_tick(chan, 0);
 }
 EXPORT_SYMBOL_GPL(mbox_client_txdone);

==================8<=================

we might also have to looking into protecting last_tx_done and tx_tick
together. Anyways, those changes are warranted in order to get the
expected functionality.

Thanks.

Re: [PATCH v5 1/8] Documentation: add DT binding for ARM System Control and Power Interface(SCPI) protocol

[Mark Rutland]

Hi,

On Thu, Jul 23, 2015 at 12:10:21PM +0100, Sudeep Holla wrote:
> This patch adds devicetree binding for System Control and Power
> Interface (SCPI) Message Protocol used between the Application Cores(AP)
> and the System Control Processor(SCP). The MHU peripheral provides a
> mechanism for inter-processor communication between SCP's M3 processor
> and AP.
> 
> SCP offers control and management of the core/cluster power states,
> various power domain DVFS including the core/cluster, certain system
> clocks configuration, thermal sensors and many others.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> CC: Jassi Brar <jassisinghbrar@gmail.com>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Jon Medhurst (Tixy) <tixy@linaro.org>
> Cc: devicetree@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/arm/arm,scpi.txt | 151 +++++++++++++++++++++
>  MAINTAINERS                                        |   6 +
>  2 files changed, 157 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/arm,scpi.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> new file mode 100644
> index 000000000000..e21cce646561
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> @@ -0,0 +1,151 @@
> +System Control and Power Interface (SCPI) Message Protocol
> +----------------------------------------------------------
> +
> +Firmware implementing the SCPI described in ARM document number ARM DUI 0922B
> +("ARM Compute Subsystem SCP: Message Interface Protocols")[0] can be used
> +by Linux to initiate various system control and power operations.
> +
> +Required properties:
> +
> +- compatible : should be "arm,scpi"
> +- mboxes: List of phandle and mailbox channel specifiers
> +	  All the channels reserved by remote SCP firmware for use by
> +	  SCPI message protocol should be specified in any order
> +- shmem : List of phandle pointing to the shared memory(SHM) area between the
> +	  processors using these mailboxes for IPC, one for each mailbox
> +	  SHM can be any memory reserved for the purpose of this communication
> +	  between the processors.
> +
> +See Documentation/devicetree/bindings/mailbox/mailbox.txt
> +for more details about the generic mailbox controller and
> +client driver bindings.
> +
> +Clock bindings for the clocks based on SCPI Message Protocol
> +------------------------------------------------------------
> +
> +This binding uses the common clock binding[1].
> +
> +Container Node
> +==============
> +Required properties:
> +- compatible : should be "arm,scpi-clocks"
> +	       All the clocks provided by SCP firmware via SCPI message
> +	       protocol much be listed as sub-nodes under this node.
> +
> +Sub-nodes
> +=========
> +Required properties:
> +- compatible : shall include one of the following
> +	"arm,scpi-dvfs-clocks" - all the clocks that are variable and index based.
> +		These clocks don't provide an entire range of values between the
> +		limits but only discrete points within the range. The firmware
> +		provides the mapping for each such operating frequency and the
> +		index associated with it. The firmware also manages the
> +		voltage scaling appropriately with the clock scaling.
> +	"arm,scpi-variable-clocks" - all the clocks that are variable and provide full
> +		range within the specified range. The firmware provides the
> +		range of values within a specified range.
> +
> +Other required properties for all clocks(all from common clock binding):
> +- #clock-cells : should be set to 1 as each of the SCPI clocks have multiple
> +	outputs.

Could you change this to:

#clock-cells: Should be 1. Contains the Clock ID value used by SCPI
              commands.

That will make it clear what value should be used when writing a dts
(the raw value commands use, as opposed to some logical abstraction
thereof).

With that:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

> +- clock-output-names : shall be the corresponding names of the outputs.
> +- clock-indices: The identifying number for the clocks(i.e.clock_id) in the
> +	node. It can be non linear and hence provide the mapping of identifiers
> +	into the clock-output-names array.
> +
> +SRAM and Shared Memory for SCPI
> +-------------------------------
> +
> +A small area of SRAM is reserved for SCPI communication between application
> +processors and SCP.
> +
> +Required properties:
> +- compatible : should be "arm,juno-sram-ns" for Non-secure SRAM on Juno
> +
> +The rest of the properties should follow the generic mmio-sram description
> +found in ../../misc/sysram.txt
> +
> +Each sub-node represents the reserved area for SCPI.
> +
> +Required sub-node properties:
> +- reg : The base offset and size of the reserved area with the SRAM
> +- compatible : should be "arm,juno-scp-shmem" for Non-secure SRAM based
> +	       shared memory on Juno platforms
> +
> +[0] http://community.arm.com/servlet/JiveServlet/download/8401-45-18326/DUI0922B_scp_message_interface.pdf
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Example:
> +
> +sram: sram@50000000 {
> +	compatible = "arm,juno-sram-ns", "mmio-sram";
> +	reg = <0x0 0x50000000 0x0 0x10000>;
> +
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	ranges = <0 0x0 0x50000000 0x10000>;
> +
> +	cpu_scp_lpri: scp-shmem@0 {
> +		compatible = "arm,juno-scp-shmem";
> +		reg = <0x0 0x200>;
> +	};
> +
> +	cpu_scp_hpri: scp-shmem@200 {
> +		compatible = "arm,juno-scp-shmem";
> +		reg = <0x200 0x200>;
> +	};
> +};
> +
> +mailbox: mailbox0@40000000 {
> +	....
> +	#mbox-cells = <1>;
> +};
> +
> +scpi_protocol: scpi@2e000000 {
> +	compatible = "arm,scpi";
> +	mboxes = <&mailbox 0 &mailbox 1>;
> +	shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
> +
> +	clocks {
> +		compatible = "arm,scpi-clocks";
> +
> +		scpi_dvfs: scpi_clocks@0 {
> +			compatible = "arm,scpi-dvfs-clocks";
> +			#clock-cells = <1>;
> +			clock-indices = <0>, <1>, <2>;
> +			clock-output-names = "atlclk", "aplclk","gpuclk";
> +		};
> +		scpi_clk: scpi_clocks@3 {
> +			compatible = "arm,scpi-variable-clocks";
> +			#clock-cells = <1>;
> +			clock-indices = <3>, <4>;
> +			clock-output-names = "pxlclk0", "pxlclk1";
> +		};
> +	};
> +};
> +
> +cpu@0 {
> +	...
> +	reg = <0 0>;
> +	clocks = <&scpi_dvfs 0>;
> +};
> +
> +hdlcd@7ff60000 {
> +	...
> +	reg = <0 0x7ff60000 0 0x1000>;
> +	clocks = <&scpi_clk 4>;
> +};
> +
> +In the above example, the #clock-cells is set to 1 as required.
> +scpi_dvfs has 3 output clocks namely: vbig, vlittle and vgpu with 0, 1
> +and 2 as clock-indices. scpi_clk has 2 output clocks namely: pxlclk0 and
> +pxlclk1 with 3 and 4 as clock-indices.
> +
> +The first consumer in the example is cpu@0 and it has '0' as the clock
> +specifier which points to the first entry in the output clocks of
> +scpi_dvfs i.e. "atlclk".
> +
> +Similarly the second example is hdlcd@7ff60000 and it has pxlclk1 as input
> +clock. '4' in the clock specifier here points to the second entry
> +in the output clocks of scpi_clocks  i.e. "pxlclk1"
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a2264167791a..9351b62dbbd7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8955,6 +8955,12 @@ W:	http://www.sunplus.com
>  S:	Supported
>  F:	arch/score/
>  
> +SYSTEM CONTROL & POWER INTERFACE (SCPI) Message Protocol drivers
> +M:	Sudeep Holla <sudeep.holla@arm.com>
> +L:	linux-arm-kernel@lists.infradead.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/arm/arm,scpi.txt
> +
>  SCSI CDROM DRIVER
>  M:	Jens Axboe <axboe@kernel.dk>
>  L:	linux-scsi@vger.kernel.org
> -- 
> 1.9.1
> 

Re: [PATCH v5 1/8] Documentation: add DT binding for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 31/07/15 17:00, Mark Rutland wrote:
> Hi,
>
> On Thu, Jul 23, 2015 at 12:10:21PM +0100, Sudeep Holla wrote:
>> This patch adds devicetree binding for System Control and Power
>> Interface (SCPI) Message Protocol used between the Application Cores(AP)
>> and the System Control Processor(SCP). The MHU peripheral provides a
>> mechanism for inter-processor communication between SCP's M3 processor
>> and AP.
>>
>> SCP offers control and management of the core/cluster power states,
>> various power domain DVFS including the core/cluster, certain system
>> clocks configuration, thermal sensors and many others.
>>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> CC: Jassi Brar <jassisinghbrar@gmail.com>
>> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Jon Medhurst (Tixy) <tixy@linaro.org>
>> Cc: devicetree@vger.kernel.org
>> ---
>>   Documentation/devicetree/bindings/arm/arm,scpi.txt | 151 +++++++++++++++++++++
>>   MAINTAINERS                                        |   6 +
>>   2 files changed, 157 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/arm/arm,scpi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> new file mode 100644
>> index 000000000000..e21cce646561
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> @@ -0,0 +1,151 @@
>> +System Control and Power Interface (SCPI) Message Protocol
>> +----------------------------------------------------------
>> +
>> +Firmware implementing the SCPI described in ARM document number ARM DUI 0922B
>> +("ARM Compute Subsystem SCP: Message Interface Protocols")[0] can be used
>> +by Linux to initiate various system control and power operations.
>> +
>> +Required properties:
>> +
>> +- compatible : should be "arm,scpi"
>> +- mboxes: List of phandle and mailbox channel specifiers
>> +	  All the channels reserved by remote SCP firmware for use by
>> +	  SCPI message protocol should be specified in any order
>> +- shmem : List of phandle pointing to the shared memory(SHM) area between the
>> +	  processors using these mailboxes for IPC, one for each mailbox
>> +	  SHM can be any memory reserved for the purpose of this communication
>> +	  between the processors.
>> +
>> +See Documentation/devicetree/bindings/mailbox/mailbox.txt
>> +for more details about the generic mailbox controller and
>> +client driver bindings.
>> +
>> +Clock bindings for the clocks based on SCPI Message Protocol
>> +------------------------------------------------------------
>> +
>> +This binding uses the common clock binding[1].
>> +
>> +Container Node
>> +==============
>> +Required properties:
>> +- compatible : should be "arm,scpi-clocks"
>> +	       All the clocks provided by SCP firmware via SCPI message
>> +	       protocol much be listed as sub-nodes under this node.
>> +
>> +Sub-nodes
>> +=========
>> +Required properties:
>> +- compatible : shall include one of the following
>> +	"arm,scpi-dvfs-clocks" - all the clocks that are variable and index based.
>> +		These clocks don't provide an entire range of values between the
>> +		limits but only discrete points within the range. The firmware
>> +		provides the mapping for each such operating frequency and the
>> +		index associated with it. The firmware also manages the
>> +		voltage scaling appropriately with the clock scaling.
>> +	"arm,scpi-variable-clocks" - all the clocks that are variable and provide full
>> +		range within the specified range. The firmware provides the
>> +		range of values within a specified range.
>> +
>> +Other required properties for all clocks(all from common clock binding):
>> +- #clock-cells : should be set to 1 as each of the SCPI clocks have multiple
>> +	outputs.
>
> Could you change this to:
>
> #clock-cells: Should be 1. Contains the Clock ID value used by SCPI
>                commands.
>

Changed now.

> That will make it clear what value should be used when writing a dts
> (the raw value commands use, as opposed to some logical abstraction
> thereof).
>

Agreed.

> With that:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>

Thanks.

Regards,
Sudeep

Re: [PATCH v5 1/8] Documentation: add DT binding for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 31/07/15 17:00, Mark Rutland wrote:
> Hi,
>
> On Thu, Jul 23, 2015 at 12:10:21PM +0100, Sudeep Holla wrote:
>> This patch adds devicetree binding for System Control and Power
>> Interface (SCPI) Message Protocol used between the Application Cores(AP)
>> and the System Control Processor(SCP). The MHU peripheral provides a
>> mechanism for inter-processor communication between SCP's M3 processor
>> and AP.
>>
>> SCP offers control and management of the core/cluster power states,
>> various power domain DVFS including the core/cluster, certain system
>> clocks configuration, thermal sensors and many others.
>>
>> Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> CC: Jassi Brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Liviu Dudau <Liviu.Dudau-5wv7dgnIgG8@public.gmane.org>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
>> Cc: Jon Medhurst (Tixy) <tixy-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> ---
>>   Documentation/devicetree/bindings/arm/arm,scpi.txt | 151 +++++++++++++++++++++
>>   MAINTAINERS                                        |   6 +
>>   2 files changed, 157 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/arm/arm,scpi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> new file mode 100644
>> index 000000000000..e21cce646561
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> @@ -0,0 +1,151 @@
>> +System Control and Power Interface (SCPI) Message Protocol
>> +----------------------------------------------------------
>> +
>> +Firmware implementing the SCPI described in ARM document number ARM DUI 0922B
>> +("ARM Compute Subsystem SCP: Message Interface Protocols")[0] can be used
>> +by Linux to initiate various system control and power operations.
>> +
>> +Required properties:
>> +
>> +- compatible : should be "arm,scpi"
>> +- mboxes: List of phandle and mailbox channel specifiers
>> +	  All the channels reserved by remote SCP firmware for use by
>> +	  SCPI message protocol should be specified in any order
>> +- shmem : List of phandle pointing to the shared memory(SHM) area between the
>> +	  processors using these mailboxes for IPC, one for each mailbox
>> +	  SHM can be any memory reserved for the purpose of this communication
>> +	  between the processors.
>> +
>> +See Documentation/devicetree/bindings/mailbox/mailbox.txt
>> +for more details about the generic mailbox controller and
>> +client driver bindings.
>> +
>> +Clock bindings for the clocks based on SCPI Message Protocol
>> +------------------------------------------------------------
>> +
>> +This binding uses the common clock binding[1].
>> +
>> +Container Node
>> +==============
>> +Required properties:
>> +- compatible : should be "arm,scpi-clocks"
>> +	       All the clocks provided by SCP firmware via SCPI message
>> +	       protocol much be listed as sub-nodes under this node.
>> +
>> +Sub-nodes
>> +=========
>> +Required properties:
>> +- compatible : shall include one of the following
>> +	"arm,scpi-dvfs-clocks" - all the clocks that are variable and index based.
>> +		These clocks don't provide an entire range of values between the
>> +		limits but only discrete points within the range. The firmware
>> +		provides the mapping for each such operating frequency and the
>> +		index associated with it. The firmware also manages the
>> +		voltage scaling appropriately with the clock scaling.
>> +	"arm,scpi-variable-clocks" - all the clocks that are variable and provide full
>> +		range within the specified range. The firmware provides the
>> +		range of values within a specified range.
>> +
>> +Other required properties for all clocks(all from common clock binding):
>> +- #clock-cells : should be set to 1 as each of the SCPI clocks have multiple
>> +	outputs.
>
> Could you change this to:
>
> #clock-cells: Should be 1. Contains the Clock ID value used by SCPI
>                commands.
>

Changed now.

> That will make it clear what value should be used when writing a dts
> (the raw value commands use, as opposed to some logical abstraction
> thereof).
>

Agreed.

> With that:
>
> Acked-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>

Thanks.

Regards,
Sudeep
--
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

Re: [PATCH v5 2/8] firmware: add support for ARM System Control and Power Interface(SCPI) protocol

[Sudeep Holla]

On 31/07/15 14:45, Jassi Brar wrote:
> On Fri, Jul 31, 2015 at 6:38 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 31/07/15 11:43, Sudeep Holla wrote:
>>>
>>>
>>>
>>> On 31/07/15 11:38, Jassi Brar wrote:
>>>>
>>>> On Fri, Jul 31, 2015 at 3:10 PM, Sudeep Holla <sudeep.holla@arm.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> I forgot to mention, we have a the following description in
>>>>> mbox_client_txdone which is misleading:
>>>>>
>>>>> "The client/protocol had received some 'ACK' packet and it notifies the
>>>>> API that the last packet was sent successfully. This only works if the
>>>>> controller can't sense TX-Done."
>>>>>
>>>>> which is clearly not the case in SCPI. IMO we may have to reword that.
>>>>>
>>>> Yes. And also see whether it could race with polling driven tx_tick.
>>>>
>>>
>>> Yes I am also looking at that now while I am trying to check if
>>> TXDONE_BY_ACK works on Juno, will keep you posted.
>>>
>>
>> OK, I recollect the racy condition now which I had in my mind from the
>> beginning convincing myself why we can't use that option. I was not good
>> in words to explain it so far but let me try with the ASCII art this
>> time. Note Tx ACK below means the remote setting the register flag and
>> not to be confused with the ACK packet. For simplicity Rx can be assumed
>> to be Tx ACK packet
>>
>>       Time       MHU/SCPI                        Remote SCP
>>                     |                                |
>>          1          |------------ Tx1 -------------->|
>>                     |                                |
>>          2          |<----------- Tx1 ACK -----------|
>>                     |                                |
>>          3          |------------ Tx2 -------------->|
>>                     |                                |
>>          4          |<----------- Rx1 ---------------|
>>                     |                                |
>>          5          |<----------- Tx2 ACK -----------|
>>                     |                                |
>>          6          |------------ Tx3 -------------->|
>>                     |                                |
>>          7          |<----------- Rx2 ---------------|
>>
>> Now lets consider the above scenario, suppose we have TXDONE_BY_ACK
>> and use mbox_client_txdone in Rx interrupt(i.e. response packet), we end
>> up in the race easily IIUC.
>>
>> E.g. A client would have sent Tx2(3) before Rx1 interrupt(4) and if we
>> ACK Tx1 now in (3), we will end up acknowledging Tx2(5) as the mailbox
>> core assumes only one Tx request at a time which is clearly not the case
>> in our setup. The client can then go ahead and send Tx3(6) overwriting
>> the payload while remote was processing or even result in remote missing
>> the request completely. Does that make sense or am I missing something ?
>>
> Yeah, that could happen. But the race can be fixed by checking
> last_tx_done if the controller provides that. If last_tx_done is not
> implemented, polling won't race.
>
> Please try the following...
>

Looks good to me. Sometimes it takes very long time(days) to hit race
conditions(esp. in firmware), so I need some time to think before
I cook up a patch to start stress test this on Juno so that I don't
waste time waiting for result.

Regards,
Sudeep