[RFC PATCH 0/5] Allwinner MMC firmware clocks implementation

[RFC PATCH 0/5] Allwinner MMC firmware clocks implementation

[Andre Przywara]

Hi,

this is a proof-of-concept series to demonstrate the usage of firmware
driven clocks using the SCPI protocol for Allwinner SoCs.
This aims to replace the tricky and highly SoC specific clocks drivers
that every new Allwinner SoC seems to require.
The idea is to abstract the internal clock specific code by the rather
generic SCPI interface, which support programming and reading clock
frequencies in an SoC agnostic manner. The actual clock access can be
put in firmware, which is by definition SoC specific and thus can
be easier and much quicker adapted to support a new SoC.
The SCPI interface requires a mailbox and a shared memory region to
send commands and receive results.

This series introduces a new mailbox driver, which uses smc instructions
to trigger a mailbox. This allows the SCPI handlers to live as a runtime
service component in the existing ARM Trusted Firmware port. Beside
being able to easily add those handlers to the existing firmware code
this approach has the advantage of avoiding building, maintaining and
loading yet another firmware component. The management processor on the
A64 uses an OpenRISC core, which requires a not fully upstream supported
toolchain. Also this core can remain free to other tasks, like offloading
of realtime-sensitive tasks.
A future implementation may later revisit this decision, adding a proper
mailbox driver and implement the SCPI handler on the OpenRISC core, the
only change needed would be to swap the mailbox node in the DT then.

Another added value is that SCPI brings us sensors and regulators
support, using this very same interface. All that's needed is to add
some firmware code to read the THS register or poke the AXP, for
instance (which ATF does already anyway) and advertise those via a DT
node. There would be no Linux changes needed for that.

As an example this series adds MMC support on top of the basic support
bit posted recently [1].
We use the existing sunxi MMC driver for the purpose of this series,
although it is known to not fully support the new MMC controller in the
A64 chip. I found the support sufficient enough to drive SD cards, though.

Patch 1/5 introduces the SMC mailbox driver, while patch 2/5 adds the
associated DT binding documentation. Patch 3/5 adds the basic SCPI nodes
to the SoC .dtsi, which patch 4/5 complements with the MMC nodes,
referencing the new firmware clocks.
The final patch 5/5 enables the SD card for the two boards which we
currently support.

This series goes on top of the v4 A64 support series [1], which itself
is based on v4.8-rc1.
It allows booting and running a userland from a micro SD card on both
the Pine64 and the BananaPi M64 board.
The matching firmware implementation can be found in the allwinner-scpi
branch here [2]. The clock code in there is probably pretty stupid, but
works and is good enough to at least serve as a proof-of-concept for this
new clock approach.

Please have a look and give comments, I am particularily interested in
how you like this idea. One motiviation is to save the kernel from
endless variations of clock driver code, also to be able to support
newer SoCs much easier without having to submit and maintain tedious
clock patches.

Cheers,
Andre.

[1]: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-August/447512.html
[2]: https://github.com/apritzel/arm-trusted-firmware/commits/allwinner-scpi

Andre Przywara (5):
  mailbox: introduce ARM SMC based mailbox
  DT: mailbox: add binding doc for the ARM SMC mailbox
  arm64: dts: sunxi: add SCPI node to sun50i-a64.dtsi
  arm64: dts: sunxi: add SCPI driven clocks and nodes for A64 MMC
  arm64: dts: sunxi: add MMC nodes to Pine64 and BPi-M64 .dts

 .../devicetree/bindings/mailbox/arm-smc.txt        |  53 ++++++++
 .../boot/dts/allwinner/sun50i-a64-bananapi-m64.dts |  29 +++++
 .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts |  20 +++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi      |  87 +++++++++++++
 drivers/mailbox/Kconfig                            |   8 ++
 drivers/mailbox/Makefile                           |   2 +
 drivers/mailbox/smc-mailbox.c                      | 135 +++++++++++++++++++++
 7 files changed, 334 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
 create mode 100644 drivers/mailbox/smc-mailbox.c

-- 
2.9.0

[RFC PATCH 1/5] mailbox: introduce ARM SMC based mailbox

[Andre Przywara]

This mailbox driver implements a mailbox which signals transmitted data
via an ARM smc (secure monitor call) instruction. The mailbox receiver
is implemented in firmware and can synchronously return data when it
returns execution to the non-secure OS again.
An asynchronous receive path is not implemented.
This allows the usage of a mailbox to trigger firmware actions on SoCs
which either don't have a separate management processor or on which such
a core is not available. A user of this mailbox could be the SCP
interface.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/mailbox/Kconfig       |   8 +++
 drivers/mailbox/Makefile      |   2 +
 drivers/mailbox/smc-mailbox.c | 135 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+)
 create mode 100644 drivers/mailbox/smc-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 97c3729..7ffaef6 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -132,4 +132,12 @@ config BCM_PDC_MBOX
 	  Mailbox implementation for the Broadcom PDC ring manager,
 	  which provides access to various offload engines on Broadcom
 	  SoCs. Say Y here if you want to use the Broadcom PDC.
+
+config SMC_MBOX
+	tristate "Generic ARM SMC mailbox"
+	depends on HAVE_ARM_SMCCC
+	depends on OF
+	help
+	  Generic mailbox driver which uses ARM smc calls to call into
+	  firmware for triggering mailboxes.
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 66c38e3..8488afd 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -27,3 +27,5 @@ obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o
 obj-$(CONFIG_HI6220_MBOX)	+= hi6220-mailbox.o
 
 obj-$(CONFIG_BCM_PDC_MBOX)	+= bcm-pdc-mailbox.o
+
+obj-$(CONFIG_SMC_MBOX)		+= smc-mailbox.o
diff --git a/drivers/mailbox/smc-mailbox.c b/drivers/mailbox/smc-mailbox.c
new file mode 100644
index 0000000..63f09bd
--- /dev/null
+++ b/drivers/mailbox/smc-mailbox.c
@@ -0,0 +1,135 @@
+/*
+ *  Copyright (C) 2016 ARM Ltd.
+ *
+ * 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 device provides a mechanism for emulating a mailbox by using
+ * smc calls, allowing a "mailbox" consumer to sit in firmware running
+ * on the same core.
+ */
+
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/arm-smccc.h>
+
+static int smc_send_data(struct mbox_chan *link, void *data)
+{
+	u32 function_id = (unsigned long)link->con_priv;
+	u32 msg = *(u32 *)data;
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(function_id, msg, 0, 0, 0, 0, 0, 0, &res);
+
+	mbox_chan_received_data(link, (void *)res.a0);
+
+	return 0;
+}
+
+static int smc_startup(struct mbox_chan *link)
+{
+	return 0;
+}
+
+static void smc_shutdown(struct mbox_chan *link)
+{
+}
+
+/* This mailbox is synchronous, so we are always done. */
+static bool smc_last_tx_done(struct mbox_chan *link)
+{
+	return true;
+}
+
+static const struct mbox_chan_ops smc_mbox_chan_ops = {
+	.send_data	= smc_send_data,
+	.startup	= smc_startup,
+	.shutdown	= smc_shutdown,
+	.last_tx_done	= smc_last_tx_done
+};
+
+static int smc_mbox_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mbox_controller *mbox;
+	int ret = 0;
+	int i;
+
+	ret = of_property_count_elems_of_size(dev->of_node, "identifiers",
+					      sizeof(u32));
+	if (ret < 0)
+		return ret;
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (mbox == NULL)
+		return -ENOMEM;
+
+	mbox->num_chans = ret;
+	mbox->chans = devm_kmalloc_array(dev, mbox->num_chans,
+					 sizeof(*mbox->chans),
+					 GFP_KERNEL | __GFP_ZERO);
+	if (!mbox->chans)
+		return -ENOMEM;
+
+	for (i = 0; i < mbox->num_chans; i++) {
+		u32 function_id;
+
+		ret = of_property_read_u32_index(dev->of_node, "identifiers", i,
+						 &function_id);
+		if (ret)
+			return ret;
+		mbox->chans[i].con_priv = (void *)(unsigned long)function_id;
+	}
+
+	mbox->txdone_poll = true;
+	mbox->txdone_irq = false;
+	mbox->txpoll_period = 1;
+	mbox->ops = &smc_mbox_chan_ops;
+	mbox->dev = dev;
+
+	ret = mbox_controller_register(mbox);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, mbox);
+	dev_info(dev, "ARM SMC mailbox enabled with %d chans.\n",
+		 mbox->num_chans);
+
+	return ret;
+}
+
+static int smc_mbox_remove(struct platform_device *pdev)
+{
+	struct mbox_controller *mbox = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(mbox);
+	return 0;
+}
+
+static const struct of_device_id smc_mbox_of_match[] = {
+	{ .compatible = "arm,smc-mbox", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, smc_mbox_of_match);
+
+static struct platform_driver smc_mbox_driver = {
+	.driver = {
+		.name = "smc-mbox",
+		.of_match_table = smc_mbox_of_match,
+	},
+	.probe		= smc_mbox_probe,
+	.remove		= smc_mbox_remove,
+};
+module_platform_driver(smc_mbox_driver);
+
+MODULE_AUTHOR("Andre Przywara <andre.przywara@arm.com>");
+MODULE_DESCRIPTION("Generic ARM smc mailbox driver");
+MODULE_LICENSE("GPL v2");
-- 
2.9.0

[RFC PATCH 2/5] DT: mailbox: add binding doc for the ARM SMC mailbox

[Andre Przywara]

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../devicetree/bindings/mailbox/arm-smc.txt        | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt

diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
new file mode 100644
index 0000000..9919a12
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
@@ -0,0 +1,53 @@
+ARM SMC Mailbox Driver
+======================
+
+This mailbox driver uses the ARM smc (secure monitor call) instruction to
+trigger a mailbox-connected activity in firmware running on the very same
+core as the caller. By nature this operation is synchronous and this
+driver provides no way for asynchronous messages to be delivered the other
+way round, from firmware to the OS. However the value of r0/w0 the firmware
+returns after the smc call is delivered as a received message to the
+mailbox framework, so a synchronous communication can be established.
+
+One usecase of this mailbox is the SCP interface, which uses shared memory
+to transfer commands and parameters and mailboxes to trigger a function
+call. This driver allows SoC without a separate management processor (or
+when such a processor is not available or used) to use this standardized
+interface anyway.
+
+The driver requires no special hardware, any core which supports the SMC
+instruction can be used. This requires firmware in monitor mode/EL3 to
+handle the mailbox message.
+
+Mailbox Device Node:
+====================
+
+Required properties:
+--------------------
+- compatible:		Shall be "arm,smc-mbox"
+- #mbox-cells		Shall be 1 - the index of the channel needed.
+- identifiers		An array of 32-bit values specifying the function
+			IDs used by each mailbox channel. Those function IDs
+			follow the ARM SMC calling convention standard [1].
+			There is one identifier per channel and the number
+			of supported channels is determined by the length
+			of this array.
+
+Example:
+--------
+
+	mailbox: smc_mbox {
+		#mbox-cells = <1>;
+		compatible = "arm,smc-mbox";
+		identifiers = <0x82000001 0x82000002>;
+	};
+
+	scpi {
+		compatible = "arm,scpi";
+		mboxes = <&mailbox 0>;
+		shmem = <&cpu_scp_shmem>;
+	};
+
+
+[1]
+http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0028a/index.html
-- 
2.9.0

[RFC PATCH 3/5] arm64: dts: sunxi: add SCPI node to sun50i-a64.dtsi

[Andre Przywara]

Support for variable frequency clocks is implemented in ARM Trusted
Firmware, which sits in SRAM and waits for SCPI requests.
Add the respective SMC mailbox node and a 512-byte chunk of SRAM to
allow SCPI calls to be handled by the firmware.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 70d0382..9fc540e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -131,8 +131,34 @@
 			(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 	};
 
+	mailbox: mbox@0 {
+		compatible = "arm,smc-mbox";
+		#mbox-cells = <1>;
+		identifiers = <0x82000001>;
+	};
+
+	sram: sram@18000{
+		compatible = "mmio-sram";
+		reg = <0x10000 0x8000>;
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x10000 0x8000>;
+
+		cpu_scp_mem: scp-shmem@7e00 {
+			compatible = "mmio-sram";
+			reg = <0x7e00 0x200>;
+		};
+	};
+
 	/include/ "sun50i-a64-clocks.dtsi"
 
+	scpi {
+		compatible = "arm,scpi";
+		mboxes = <&mailbox 0>;
+		shmem = <&cpu_scp_mem>;
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <1>;
-- 
2.9.0

[RFC PATCH 4/5] arm64: dts: sunxi: add SCPI driven clocks and nodes for A64 MMC

[Andre Przywara]

The MMC controllers in the Allwinner A64 SoC are somewhat compatible
with the versions used in other Allwinner SoCs.
Tell Linux about the three MMC clocks that the firmware implements and
add nodes to represent the MMC controllers.
The actual hardware is capable of new transfer modes, which the driver
does not fully support yet, also the clock part has changed, but it
works like this at least for SD card accesses.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 61 +++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 9fc540e..0f6044b 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -157,6 +157,19 @@
 		compatible = "arm,scpi";
 		mboxes = <&mailbox 0>;
 		shmem = <&cpu_scp_mem>;
+
+		clocks {
+			compatible = "arm,scpi-clocks";
+
+			scpi_clk: scpi_clocks {
+				compatible = "arm,scpi-variable-clocks";
+				#clock-cells = <1>;
+				clock-indices = <0>, <1>,
+						<2>;
+				clock-output-names = "mmc0_clk", "mmc1_clk",
+						     "mmc2_clk";
+			};
+		};
 	};
 
 	soc {
@@ -165,6 +178,54 @@
 		#size-cells = <1>;
 		ranges;
 
+		mmc0: mmc@1c0f000 {
+			compatible = "allwinner,sun50i-a64-mmc",
+				     "allwinner,sun5i-a13-mmc";
+			reg = <0x01c0f000 0x1000>;
+			clocks = <&bus_gates 8>, <&scpi_clk 0>,
+				 <&scpi_clk 0>, <&scpi_clk 0>;
+			clock-names = "ahb", "mmc",
+				      "output", "sample";
+			resets = <&ahb_rst 8>;
+			reset-names = "ahb";
+			interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		mmc1: mmc@1c10000 {
+			compatible = "allwinner,sun50i-a64-mmc",
+				     "allwinner,sun5i-a13-mmc";
+			reg = <0x01c10000 0x1000>;
+			clocks = <&bus_gates 9>, <&scpi_clk 1>,
+				 <&scpi_clk 1>, <&scpi_clk 1>;
+			clock-names = "ahb", "mmc",
+				      "output", "sample";
+			resets = <&ahb_rst 9>;
+			reset-names = "ahb";
+			interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		mmc2: mmc@1c11000 {
+			compatible = "allwinner,sun50i-a64-mmc",
+				     "allwinner,sun5i-a13-mmc";
+			reg = <0x01c11000 0x1000>;
+			clocks = <&bus_gates 10>, <&scpi_clk 2>,
+				 <&scpi_clk 2>, <&scpi_clk 2>;
+			clock-names = "ahb", "mmc",
+				      "output", "sample";
+			resets = <&ahb_rst 10>;
+			reset-names = "ahb";
+			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		pio: pinctrl@1c20800 {
 			compatible = "allwinner,sun50i-a64-pinctrl";
 			reg = <0x01c20800 0x400>;
-- 
2.9.0

[RFC PATCH 5/5] arm64: dts: sunxi: add MMC nodes to Pine64 and BPi-M64 .dts

[Andre Przywara]

Those two boards connect a microSD card slot to the MMC0 controller.
Add the dummy regulator and enable MMC0 to allow accessing the SD card.

The BananaPi M64 has an on-board eMMC chip connected to the MMC2
controller, but the existing MMC driver does not support this properly
yet, so keep this one disabled for now.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../boot/dts/allwinner/sun50i-a64-bananapi-m64.dts | 29 ++++++++++++++++++++++
 .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 20 +++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
index bc0ed4c..f98c351 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
@@ -59,6 +59,35 @@
 	aliases {
 		serial0 = &uart0;
 	};
+
+	soc {
+		reg_vcc3v3: vcc3v3 {
+			compatible = "regulator-fixed";
+			regulator-name = "vcc3v3";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+		};
+	};
+};
+
+&mmc0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc0_pins>, <&mmc0_default_cd_pin>;
+	bus-width = <4>;
+	vmmc-supply = <&reg_vcc3v3>;
+	cd-gpios = <&pio 5 6 0>;
+	cd-inverted;
+	disable-wp;
+	status = "okay";
+};
+
+&mmc2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc2_pins>;
+	bus-width = <8>;
+	vmmc-supply = <&reg_vcc3v3>;
+	non-removable;
+	status = "disabled";
 };
 
 &uart0 {
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index 077a56f..a3957ca 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -59,6 +59,26 @@
 	aliases {
 		serial0 = &uart0;
 	};
+
+	soc {
+		reg_vcc3v3: vcc3v3 {
+			compatible = "regulator-fixed";
+			regulator-name = "vcc3v3";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+		};
+	};
+};
+
+&mmc0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc0_pins>, <&mmc0_default_cd_pin>;
+	bus-width = <4>;
+	vmmc-supply = <&reg_vcc3v3>;
+	cd-gpios = <&pio 5 6 0>;
+	cd-inverted;
+	disable-wp;
+	status = "okay";
 };
 
 &uart0 {
-- 
2.9.0

Re: [RFC PATCH 0/5] Allwinner MMC firmware clocks implementation

[Stefan Monnier]

> frequencies in an SoC agnostic manner. The actual clock access can be
> put in firmware, which is by definition SoC specific and thus can
> be easier and much quicker adapted to support a new SoC.

Hmm... what do you mean by "firmware"?


        Stefan "just a by-stander trying to understand"

Re: [linux-sunxi] [RFC PATCH 1/5] mailbox: introduce ARM SMC based mailbox

[LABBE Corentin]

Hello

Just a minor comment below

> +static int smc_mbox_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mbox_controller *mbox;
> +	int ret = 0;
> +	int i;
> +
> +	ret = of_property_count_elems_of_size(dev->of_node, "identifiers",
> +					      sizeof(u32));
> +	if (ret < 0)
> +		return ret;
> +
> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> +	if (mbox == NULL)
> +		return -ENOMEM;

checkpatch prefer !mbox

> +
> +	mbox->num_chans = ret;
> +	mbox->chans = devm_kmalloc_array(dev, mbox->num_chans,
> +					 sizeof(*mbox->chans),
> +					 GFP_KERNEL | __GFP_ZERO);

devm_kcalloc seems to do the same

Regards

LABBE Corentin

Re: [RFC PATCH 2/5] DT: mailbox: add binding doc for the ARM SMC mailbox

[Rob Herring]

On Tue, Aug 09, 2016 at 12:53:00PM +0100, Andre Przywara wrote:
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../devicetree/bindings/mailbox/arm-smc.txt        | 53 ++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> new file mode 100644
> index 0000000..9919a12
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> @@ -0,0 +1,53 @@
> +ARM SMC Mailbox Driver
> +======================
> +
> +This mailbox driver uses the ARM smc (secure monitor call) instruction to
> +trigger a mailbox-connected activity in firmware running on the very same
> +core as the caller. By nature this operation is synchronous and this
> +driver provides no way for asynchronous messages to be delivered the other
> +way round, from firmware to the OS. However the value of r0/w0 the firmware
> +returns after the smc call is delivered as a received message to the
> +mailbox framework, so a synchronous communication can be established.

Clever

> +
> +One usecase of this mailbox is the SCP interface, which uses shared memory
> +to transfer commands and parameters and mailboxes to trigger a function
> +call. This driver allows SoC without a separate management processor (or
> +when such a processor is not available or used) to use this standardized
> +interface anyway.
> +
> +The driver requires no special hardware, any core which supports the SMC
> +instruction can be used. This requires firmware in monitor mode/EL3 to
> +handle the mailbox message.
> +
> +Mailbox Device Node:
> +====================
> +
> +Required properties:
> +--------------------
> +- compatible:		Shall be "arm,smc-mbox"
> +- #mbox-cells		Shall be 1 - the index of the channel needed.
> +- identifiers		An array of 32-bit values specifying the function

Minimally, needs 'arm' prefix. 'identifiers' is pretty vague. Perhaps 
'arm,smc-func-ids' to say exactly what they are.

> +			IDs used by each mailbox channel. Those function IDs
> +			follow the ARM SMC calling convention standard [1].
> +			There is one identifier per channel and the number
> +			of supported channels is determined by the length
> +			of this array.
> +
> +Example:
> +--------
> +
> +	mailbox: smc_mbox {
> +		#mbox-cells = <1>;
> +		compatible = "arm,smc-mbox";
> +		identifiers = <0x82000001 0x82000002>;
> +	};
> +
> +	scpi {
> +		compatible = "arm,scpi";
> +		mboxes = <&mailbox 0>;
> +		shmem = <&cpu_scp_shmem>;
> +	};
> +
> +
> +[1]
> +http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0028a/index.html
> -- 
> 2.9.0
> 

Re: [RFC PATCH 0/5] Allwinner MMC firmware clocks implementation

[André Przywara]

On 10/08/16 16:10, Icenowy Zheng wrote:
> 
> 
> 09.08.2016, 19:57, "Andre Przywara" <andre.przywara@arm.com>:
>> Hi,
>>
>> this is a proof-of-concept series to demonstrate the usage of firmware
>> driven clocks using the SCPI protocol for Allwinner SoCs.
>> This aims to replace the tricky and highly SoC specific clocks drivers
>> that every new Allwinner SoC seems to require.
>> The idea is to abstract the internal clock specific code by the rather
>> generic SCPI interface, which support programming and reading clock
>> frequencies in an SoC agnostic manner. The actual clock access can be
>> put in firmware, which is by definition SoC specific and thus can
>> be easier and much quicker adapted to support a new SoC.
>> The SCPI interface requires a mailbox and a shared memory region to
>> send commands and receive results.
>>
>> This series introduces a new mailbox driver, which uses smc instructions
>> to trigger a mailbox. This allows the SCPI handlers to live as a runtime
>> service component in the existing ARM Trusted Firmware port. Beside
>> being able to easily add those handlers to the existing firmware code
>> this approach has the advantage of avoiding building, maintaining and
>> loading yet another firmware component. The management processor on the
>> A64 uses an OpenRISC core, which requires a not fully upstream supported
>> toolchain. Also this core can remain free to other tasks, like offloading
>> of realtime-sensitive tasks.
> 
> Here's a problem...
> 
> Although SCPI is a standard, SMC mailbox and ATF clock id is not.
> 
> And there's currently no generic clock framework in ATF...

And I don't see why there is a need for that.
More below.

> 
>> A future implementation may later revisit this decision, adding a proper
>> mailbox driver and implement the SCPI handler on the OpenRISC core, the
>> only change needed would be to swap the mailbox node in the DT then.
>>
>> Another added value is that SCPI brings us sensors and regulators
>> support, using this very same interface. All that's needed is to add
>> some firmware code to read the THS register or poke the AXP, for
>> instance (which ATF does already anyway) and advertise those via a DT
>> node. There would be no Linux changes needed for that.
>>
>> As an example this series adds MMC support on top of the basic support
>> bit posted recently [1].
>> We use the existing sunxi MMC driver for the purpose of this series,
>> although it is known to not fully support the new MMC controller in the
>> A64 chip. I found the support sufficient enough to drive SD cards, though.
>>
>> Patch 1/5 introduces the SMC mailbox driver, while patch 2/5 adds the
>> associated DT binding documentation. Patch 3/5 adds the basic SCPI nodes
>> to the SoC .dtsi, which patch 4/5 complements with the MMC nodes,
>> referencing the new firmware clocks.
>> The final patch 5/5 enables the SD card for the two boards which we
>> currently support.
>>
>> This series goes on top of the v4 A64 support series [1], which itself
>> is based on v4.8-rc1.
>> It allows booting and running a userland from a micro SD card on both
>> the Pine64 and the BananaPi M64 board.
>> The matching firmware implementation can be found in the allwinner-scpi
>> branch here [2]. The clock code in there is probably pretty stupid, but
>> works and is good enough to at least serve as a proof-of-concept for this
>> new clock approach.
>>
>> Please have a look and give comments, I am particularily interested in
>> how you like this idea. One motiviation is to save the kernel from
>> endless variations of clock driver code, also to be able to support
>> newer SoCs much easier without having to submit and maintain tedious
>> clock patches.
>>
>> Cheers,
>> Andre.
>>
>> [1]: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-August/447512.html
>> [2]: https://github.com/apritzel/arm-trusted-firmware/commits/allwinner-scpi
> 
> The code seems to be difficult to extend...

This is just proof of concept code hacked together mostly in one night.
I just didn't want to delay this any more and so followed "Release
early, release often" here.
Once I add a second clock, I will probably rework it anyway.

> I hope you can make the SMC mailbox a standard interface,

I don't know if it needs to be some kind of "standard interface". We
follow the ARM SMC Calling Convention on one end and plug to the
existing Linux mailbox framework (which SCPI depends on) on the other,
advertising this via DT.
So all we need to take care of is upstreaming this.

> and add a common
> clock controlling system in the ATF...

Why is this needed? For now this is just another vendor specified SMC
call, advertised via DT. Other SoC vendors have those already in
upstream ATF (Mediatek, Rockchip).
If we confine these clock setups to one particular platform (or even
SoC), the code can be quite simple.

> (And maybe add A64 as part of the reference implemention)

Possibly, but right now I don't see a need for a bullet-proof and
upstream-accepted framework implementation - as long as we comply with a
standard interface like SCPI. Actually we can swap the implementation at
any time, as long as we stay standards compliant.
As mentioned above, this particular ATF implementation was more meant as
a proof of concept.

> (but I think it may be to difficult to you to make standards and change
> reference implemention...)

I don't have the impression that ATF is reluctant to those changes.
And which standards would I need to "make"? SCPI is already out there,
and adding platform specific code is normal and expected for a firmware
implementation. There is an extra design document [1] which describes
how to add vendor specific smc calls:
https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/rt-svc-writers-guide.md

Cheers,
Andre.

> 
>>
>> Andre Przywara (5):
>>   mailbox: introduce ARM SMC based mailbox
>>   DT: mailbox: add binding doc for the ARM SMC mailbox
>>   arm64: dts: sunxi: add SCPI node to sun50i-a64.dtsi
>>   arm64: dts: sunxi: add SCPI driven clocks and nodes for A64 MMC
>>   arm64: dts: sunxi: add MMC nodes to Pine64 and BPi-M64 .dts
>>
>>  .../devicetree/bindings/mailbox/arm-smc.txt | 53 ++++++++
>>  .../boot/dts/allwinner/sun50i-a64-bananapi-m64.dts | 29 +++++
>>  .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 20 +++
>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 87 +++++++++++++
>>  drivers/mailbox/Kconfig | 8 ++
>>  drivers/mailbox/Makefile | 2 +
>>  drivers/mailbox/smc-mailbox.c | 135 +++++++++++++++++++++
>>  7 files changed, 334 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
>>  create mode 100644 drivers/mailbox/smc-mailbox.c
>>
>> --
>> 2.9.0
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [RFC PATCH 4/5] arm64: dts: sunxi: add SCPI driven clocks and nodes for A64 MMC

[André Przywara]

On 10/08/16 16:01, Icenowy Zheng wrote:

Hi,

> 09.08.2016, 19:58, "Andre Przywara" <andre.przywara@arm.com>:
>>  The MMC controllers in the Allwinner A64 SoC are somewhat compatible
>>  with the versions used in other Allwinner SoCs.
>>  Tell Linux about the three MMC clocks that the firmware implements and
>>  add nodes to represent the MMC controllers.
>>  The actual hardware is capable of new transfer modes, which the driver
>>  does not fully support yet, also the clock part has changed, but it
>>  works like this at least for SD card accesses.
>>
>>  Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>  ---
>>   arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 61 +++++++++++++++++++++++++++
>>   1 file changed, 61 insertions(+)
>>
>>  diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>>  index 9fc540e..0f6044b 100644
>>  --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>>  +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>>  @@ -157,6 +157,19 @@
>>                   compatible = "arm,scpi";
>>                   mboxes = <&mailbox 0>;
>>                   shmem = <&cpu_scp_mem>;
>>  +
>>  + clocks {
>>  + compatible = "arm,scpi-clocks";
>>  +
>>  + scpi_clk: scpi_clocks {
>>  + compatible = "arm,scpi-variable-clocks";
>>  + #clock-cells = <1>;
>>  + clock-indices = <0>, <1>,
> 
> 
> I found a problem of the "standardize" process.
> There cannot be an authority to keep the clock ID standardized, in both 

Why would this need to be standardized? All we would need to take care
of is matching firmware and DT. Ideally firmware generates (or provides)
that part of the DT, so it just puts in the bits and IDs it actually
implements.
Actually I am more for exploiting the "name" field for a clock that SCPI
provides. It allows clocks to be identified without knowing any ID in
advance, up to the point where we have something like "Ethernet" as a
clock identifier. I wonder if Linux could make use of that somehow.

Cheers,
Andre

> 
>>  + <2>;
>>  + clock-output-names = "mmc0_clk", "mmc1_clk",
>>  + "mmc2_clk";
>>  + };
>>  + };
>>           };
>>
>>           soc {
>>  @@ -165,6 +178,54 @@
>>                   #size-cells = <1>;
>>                   ranges;
>>
>>  + mmc0: mmc@1c0f000 {
>>  + compatible = "allwinner,sun50i-a64-mmc",
>>  + "allwinner,sun5i-a13-mmc";
>>  + reg = <0x01c0f000 0x1000>;
>>  + clocks = <&bus_gates 8>, <&scpi_clk 0>,
>>  + <&scpi_clk 0>, <&scpi_clk 0>;
>>  + clock-names = "ahb", "mmc",
>>  + "output", "sample";
>>  + resets = <&ahb_rst 8>;
>>  + reset-names = "ahb";
>>  + interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
>>  + status = "disabled";
>>  + #address-cells = <1>;
>>  + #size-cells = <0>;
>>  + };
>>  +
>>  + mmc1: mmc@1c10000 {
>>  + compatible = "allwinner,sun50i-a64-mmc",
>>  + "allwinner,sun5i-a13-mmc";
>>  + reg = <0x01c10000 0x1000>;
>>  + clocks = <&bus_gates 9>, <&scpi_clk 1>,
>>  + <&scpi_clk 1>, <&scpi_clk 1>;
>>  + clock-names = "ahb", "mmc",
>>  + "output", "sample";
>>  + resets = <&ahb_rst 9>;
>>  + reset-names = "ahb";
>>  + interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
>>  + status = "disabled";
>>  + #address-cells = <1>;
>>  + #size-cells = <0>;
>>  + };
>>  +
>>  + mmc2: mmc@1c11000 {
>>  + compatible = "allwinner,sun50i-a64-mmc",
>>  + "allwinner,sun5i-a13-mmc";
>>  + reg = <0x01c11000 0x1000>;
>>  + clocks = <&bus_gates 10>, <&scpi_clk 2>,
>>  + <&scpi_clk 2>, <&scpi_clk 2>;
>>  + clock-names = "ahb", "mmc",
>>  + "output", "sample";
>>  + resets = <&ahb_rst 10>;
>>  + reset-names = "ahb";
>>  + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
>>  + status = "disabled";
>>  + #address-cells = <1>;
>>  + #size-cells = <0>;
>>  + };
>>  +
>>                   pio: pinctrl@1c20800 {
>>                           compatible = "allwinner,sun50i-a64-pinctrl";
>>                           reg = <0x01c20800 0x400>;
>>  --
>>  2.9.0
>>
>>  _______________________________________________
>>  linux-arm-kernel mailing list
>>  linux-arm-kernel@lists.infradead.org
>>  http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>