linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/10] Allwinner sunxi message box support
@ 2019-08-20  3:23 Samuel Holland
  2019-08-20  3:23 ` [PATCH v4 01/10] clk: sunxi-ng: Mark msgbox clocks as critical Samuel Holland
                   ` (10 more replies)
  0 siblings, 11 replies; 29+ messages in thread
From: Samuel Holland @ 2019-08-20  3:23 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick
  Cc: devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Samuel Holland

This series adds support for the "hardware message box" in sun8i, sun9i,
and sun50i SoCs, used for communication with the ARISC management
processor (the platform's equivalent of the ARM SCP). The end goal is to
use the arm_scpi driver as a client, communicating with firmware running
on the AR100 CPU, or to use the mailbox to forward NMIs that the
firmware picks up from R_INTC.

Unfortunately, the ARM SCPI client no longer works with this driver
since it now exposes all 8 hardware FIFOs individually. The SCPI client
could be made to work (and I posted proof-of-concept code to that effect
with v1 of this series), but that is a low priority, as Linux does not
directly use SCPI with the current firmware version; all SCPI use goes
through ATF via PSCI.

As requested in the comments to v3 of this patchset, a demo client is
provided in the final patch. This demo goes along with a toy firmware
which shows that the driver does indeed work for two-way communication
on all channels. To build the firmware component, run:

  git clone https://github.com/crust-firmware/meta meta
  git clone -b mailbox-demo https://github.com/crust-firmware/crust meta/crust
  cd meta
  make

That will by default produce a U-Boot + ATF + SCP firmware image in
[meta/]build/pinebook/u-boot-sunxi-with-spl.bin. See the top-level
README.md for more information, such as cross-compiler setup.

I've now used this driver with three separate clients over the past two
years, and they all work. If there are no remaining concerns with the
driver, I'd like it to get merged.

Even without the driver, the clock patches (1-2) can go in at any time.

Changes from v3:
  - Rebased on sunxi-next
  - Added Rob's Reviewed-by for patch 3
  - Fixed a crash when receiving a message on a disabled channel
  - Cleaned up some comments/formatting in the driver
  - Fixed #mbox-cells in sunxi-h3-h5.dtsi (patch 7)
  - Removed the irqchip example (no longer relevant to the fw design)
  - Added a demo/example client that uses the driver and a toy firmware

Changes from v2:
  - Merge patches 1-3
  - Add a comment in the code explaining the CLK_IS_CRITICAL usage
  - Add a patch to mark the AR100 clocks as critical
  - Use YAML for the device tree binding
  - Include a not-for-merge example usage of the mailbox

Changes from v1:
  - Marked message box clocks as critical instead of hacks in the driver
  - 8 unidirectional channels instead of 4 bidirectional pairs
  - Use per-SoC compatible strings and an A31 fallback compatible
  - Dropped the mailbox framework patch
  - Include DT patches for SoCs that document the message box

Samuel Holland (10):
  clk: sunxi-ng: Mark msgbox clocks as critical
  clk: sunxi-ng: Mark AR100 clocks as critical
  dt-bindings: mailbox: Add a sunxi message box binding
  mailbox: sunxi-msgbox: Add a new mailbox driver
  ARM: dts: sunxi: a80: Add msgbox node
  ARM: dts: sunxi: a83t: Add msgbox node
  ARM: dts: sunxi: h3/h5: Add msgbox node
  arm64: dts: allwinner: a64: Add msgbox node
  arm64: dts: allwinner: h6: Add msgbox node
  [DO NOT MERGE] drivers: firmware: msgbox demo

 .../mailbox/allwinner,sunxi-msgbox.yaml       |  79 +++++
 arch/arm/boot/dts/sun8i-a83t.dtsi             |  10 +
 arch/arm/boot/dts/sun9i-a80.dtsi              |  10 +
 arch/arm/boot/dts/sunxi-h3-h5.dtsi            |  10 +
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  34 ++
 arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  24 ++
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  10 +
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c         |   3 +-
 drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c        |   2 +-
 drivers/clk/sunxi-ng/ccu-sun50i-h6.c          |   3 +-
 drivers/clk/sunxi-ng/ccu-sun8i-a23.c          |   3 +-
 drivers/clk/sunxi-ng/ccu-sun8i-a33.c          |   3 +-
 drivers/clk/sunxi-ng/ccu-sun8i-a83t.c         |   3 +-
 drivers/clk/sunxi-ng/ccu-sun8i-h3.c           |   3 +-
 drivers/clk/sunxi-ng/ccu-sun8i-r.c            |   2 +-
 drivers/clk/sunxi-ng/ccu-sun9i-a80.c          |   3 +-
 drivers/firmware/Kconfig                      |   6 +
 drivers/firmware/Makefile                     |   1 +
 drivers/firmware/sunxi_msgbox_demo.c          | 307 +++++++++++++++++
 drivers/mailbox/Kconfig                       |  10 +
 drivers/mailbox/Makefile                      |   2 +
 drivers/mailbox/sunxi-msgbox.c                | 323 ++++++++++++++++++
 22 files changed, 842 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
 create mode 100644 drivers/firmware/sunxi_msgbox_demo.c
 create mode 100644 drivers/mailbox/sunxi-msgbox.c

-- 
2.21.0

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v4 01/10] clk: sunxi-ng: Mark msgbox clocks as critical
  2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
@ 2019-08-20  3:23 ` Samuel Holland
  2019-08-20  3:23 ` [PATCH v4 02/10] clk: sunxi-ng: Mark AR100 " Samuel Holland
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Samuel Holland @ 2019-08-20  3:23 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick
  Cc: devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Samuel Holland

The msgbox clock is critical because the hardware it controls is shared
between Linux and system firmware. The message box may be used by the
EL3 secure monitor's PSCI implementation. On 64-bit sunxi SoCs, this is
provided by ARM TF-A; 32-bit SoCs use a different implementation. The
secure monitor uses the message box to forward requests to power
management firmware running on a separate CPU.

It is not enough for the secure monitor to enable the clock each time
Linux performs a SMC into EL3, as both the firmware and Linux can run
concurrently on separate CPUs. So it is never safe for Linux to turn
this clock off, and it should be marked as critical.

At this time, such power management firmware only exists for the A64 and
H5 SoCs.  However, it makes sense to take care of all CCU drivers now
for consistency, and to ease the transition in the future once firmware
is ported to the other SoCs.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 3 ++-
 drivers/clk/sunxi-ng/ccu-sun50i-h6.c  | 3 ++-
 drivers/clk/sunxi-ng/ccu-sun8i-a23.c  | 3 ++-
 drivers/clk/sunxi-ng/ccu-sun8i-a33.c  | 3 ++-
 drivers/clk/sunxi-ng/ccu-sun8i-a83t.c | 3 ++-
 drivers/clk/sunxi-ng/ccu-sun8i-h3.c   | 3 ++-
 drivers/clk/sunxi-ng/ccu-sun9i-a80.c  | 3 ++-
 7 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index 49bd7a4c015c..045121b50da3 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -342,8 +342,9 @@ static SUNXI_CCU_GATE(bus_de_clk,	"bus-de",	"ahb1",
 		      0x064, BIT(12), 0);
 static SUNXI_CCU_GATE(bus_gpu_clk,	"bus-gpu",	"ahb1",
 		      0x064, BIT(20), 0);
+/* Used for communication between firmware components at runtime */
 static SUNXI_CCU_GATE(bus_msgbox_clk,	"bus-msgbox",	"ahb1",
-		      0x064, BIT(21), 0);
+		      0x064, BIT(21), CLK_IS_CRITICAL);
 static SUNXI_CCU_GATE(bus_spinlock_clk,	"bus-spinlock",	"ahb1",
 		      0x064, BIT(22), 0);
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
index aebef4af9861..14f39bc4180f 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
@@ -340,8 +340,9 @@ static SUNXI_CCU_GATE(bus_vp9_clk, "bus-vp9", "psi-ahb1-ahb2",
 static SUNXI_CCU_GATE(bus_dma_clk, "bus-dma", "psi-ahb1-ahb2",
 		      0x70c, BIT(0), 0);
 
+/* Used for communication between firmware components at runtime */
 static SUNXI_CCU_GATE(bus_msgbox_clk, "bus-msgbox", "psi-ahb1-ahb2",
-		      0x71c, BIT(0), 0);
+		      0x71c, BIT(0), CLK_IS_CRITICAL);
 
 static SUNXI_CCU_GATE(bus_spinlock_clk, "bus-spinlock", "psi-ahb1-ahb2",
 		      0x72c, BIT(0), 0);
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a23.c b/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
index 103aa504f6c8..5a28583f57e2 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a23.c
@@ -255,8 +255,9 @@ static SUNXI_CCU_GATE(bus_de_fe_clk,	"bus-de-fe",	"ahb1",
 		      0x064, BIT(14), 0);
 static SUNXI_CCU_GATE(bus_gpu_clk,	"bus-gpu",	"ahb1",
 		      0x064, BIT(20), 0);
+/* Used for communication between firmware components at runtime */
 static SUNXI_CCU_GATE(bus_msgbox_clk,	"bus-msgbox",	"ahb1",
-		      0x064, BIT(21), 0);
+		      0x064, BIT(21), CLK_IS_CRITICAL);
 static SUNXI_CCU_GATE(bus_spinlock_clk,	"bus-spinlock",	"ahb1",
 		      0x064, BIT(22), 0);
 static SUNXI_CCU_GATE(bus_drc_clk,	"bus-drc",	"ahb1",
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
index 91838cd11037..50cf3726ef30 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a33.c
@@ -267,8 +267,9 @@ static SUNXI_CCU_GATE(bus_de_fe_clk,	"bus-de-fe",	"ahb1",
 		      0x064, BIT(14), 0);
 static SUNXI_CCU_GATE(bus_gpu_clk,	"bus-gpu",	"ahb1",
 		      0x064, BIT(20), 0);
+/* Used for communication between firmware components at runtime */
 static SUNXI_CCU_GATE(bus_msgbox_clk,	"bus-msgbox",	"ahb1",
-		      0x064, BIT(21), 0);
+		      0x064, BIT(21), CLK_IS_CRITICAL);
 static SUNXI_CCU_GATE(bus_spinlock_clk,	"bus-spinlock",	"ahb1",
 		      0x064, BIT(22), 0);
 static SUNXI_CCU_GATE(bus_drc_clk,	"bus-drc",	"ahb1",
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
index 2b434521c5cc..4ab3a76f4ffa 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
@@ -339,8 +339,9 @@ static SUNXI_CCU_GATE(bus_de_clk,	"bus-de",	"ahb1",
 		      0x064, BIT(12), 0);
 static SUNXI_CCU_GATE(bus_gpu_clk,	"bus-gpu",	"ahb1",
 		      0x064, BIT(20), 0);
+/* Used for communication between firmware components at runtime */
 static SUNXI_CCU_GATE(bus_msgbox_clk,	"bus-msgbox",	"ahb1",
-		      0x064, BIT(21), 0);
+		      0x064, BIT(21), CLK_IS_CRITICAL);
 static SUNXI_CCU_GATE(bus_spinlock_clk,	"bus-spinlock",	"ahb1",
 		      0x064, BIT(22), 0);
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-h3.c b/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
index 6b636362379e..7429d3fe8fb7 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
@@ -273,8 +273,9 @@ static SUNXI_CCU_GATE(bus_de_clk,	"bus-de",	"ahb1",
 		      0x064, BIT(12), 0);
 static SUNXI_CCU_GATE(bus_gpu_clk,	"bus-gpu",	"ahb1",
 		      0x064, BIT(20), 0);
+/* Used for communication between firmware components at runtime */
 static SUNXI_CCU_GATE(bus_msgbox_clk,	"bus-msgbox",	"ahb1",
-		      0x064, BIT(21), 0);
+		      0x064, BIT(21), CLK_IS_CRITICAL);
 static SUNXI_CCU_GATE(bus_spinlock_clk,	"bus-spinlock",	"ahb1",
 		      0x064, BIT(22), 0);
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun9i-a80.c b/drivers/clk/sunxi-ng/ccu-sun9i-a80.c
index dcac1391767f..47d1d18b6f38 100644
--- a/drivers/clk/sunxi-ng/ccu-sun9i-a80.c
+++ b/drivers/clk/sunxi-ng/ccu-sun9i-a80.c
@@ -748,8 +748,9 @@ static SUNXI_CCU_GATE(bus_usb_clk,	"bus-usb",	"ahb1",
 		      0x584, BIT(1), 0);
 static SUNXI_CCU_GATE(bus_gmac_clk,	"bus-gmac",	"ahb1",
 		      0x584, BIT(17), 0);
+/* Used for communication between firmware components at runtime */
 static SUNXI_CCU_GATE(bus_msgbox_clk,	"bus-msgbox",	"ahb1",
-		      0x584, BIT(21), 0);
+		      0x584, BIT(21), CLK_IS_CRITICAL);
 static SUNXI_CCU_GATE(bus_spinlock_clk,	"bus-spinlock",	"ahb1",
 		      0x584, BIT(22), 0);
 static SUNXI_CCU_GATE(bus_hstimer_clk,	"bus-hstimer",	"ahb1",
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 02/10] clk: sunxi-ng: Mark AR100 clocks as critical
  2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
  2019-08-20  3:23 ` [PATCH v4 01/10] clk: sunxi-ng: Mark msgbox clocks as critical Samuel Holland
@ 2019-08-20  3:23 ` Samuel Holland
  2019-08-20  7:11   ` Maxime Ripard
  2019-08-20  3:23 ` [PATCH v4 03/10] dt-bindings: mailbox: Add a sunxi message box binding Samuel Holland
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: Samuel Holland @ 2019-08-20  3:23 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick
  Cc: devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Samuel Holland

On sun8i, sun9i, and sun50i SoCs, system suspend/resume support requires
firmware running on the AR100 coprocessor (the "SCP"). Such firmware can
provide additional features, such as thermal monitoring and poweron/off
support for boards without a PMIC.

Since the AR100 may be running critical firmware, even if Linux does not
know about it or directly interact with it (all requests may go through
an intermediary interface such as PSCI), Linux must not turn off its
clock.

At this time, such power management firmware only exists for the A64 and
H5 SoCs.  However, it makes sense to take care of all CCU drivers now
for consistency, and to ease the transition in the future once firmware
is ported to the other SoCs.

Leaving the clock running is safe even if no firmware is present, since
the AR100 stays in reset by default. In most cases, the AR100 clock is
kept enabled by Linux anyway, since it is the parent of all APB0 bus
peripherals. This change only prevents Linux from turning off the AR100
clock in the rare case that no peripherals are in use.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c | 2 +-
 drivers/clk/sunxi-ng/ccu-sun8i-r.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c b/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
index 45a1ed3fe674..adf907020951 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c
@@ -45,7 +45,7 @@ static struct ccu_div ar100_clk = {
 		.hw.init	= CLK_HW_INIT_PARENTS("ar100",
 						      ar100_r_apb2_parents,
 						      &ccu_div_ops,
-						      0),
+						      CLK_IS_CRITICAL),
 	},
 };
 
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-r.c b/drivers/clk/sunxi-ng/ccu-sun8i-r.c
index 4646fdc61053..feef4f750943 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-r.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-r.c
@@ -45,7 +45,7 @@ static struct ccu_div ar100_clk = {
 		.hw.init	= CLK_HW_INIT_PARENTS_DATA("ar100",
 							   ar100_parents,
 							   &ccu_div_ops,
-							   0),
+							   CLK_IS_CRITICAL),
 	},
 };
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 03/10] dt-bindings: mailbox: Add a sunxi message box binding
  2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
  2019-08-20  3:23 ` [PATCH v4 01/10] clk: sunxi-ng: Mark msgbox clocks as critical Samuel Holland
  2019-08-20  3:23 ` [PATCH v4 02/10] clk: sunxi-ng: Mark AR100 " Samuel Holland
@ 2019-08-20  3:23 ` Samuel Holland
  2019-08-20  7:14   ` Maxime Ripard
  2019-08-20  3:23 ` [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver Samuel Holland
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: Samuel Holland @ 2019-08-20  3:23 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick
  Cc: devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Samuel Holland, Rob Herring

This mailbox hardware is present in Allwinner sun8i, sun9i, and sun50i
SoCs. Add a device tree binding for it.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 .../mailbox/allwinner,sunxi-msgbox.yaml       | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml b/Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
new file mode 100644
index 000000000000..f34a1909ab2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/allwinner,sunxi-msgbox.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner sunxi Message Box
+
+maintainers:
+  - Samuel Holland <samuel@sholland.org>
+
+description: |
+  The hardware message box on sun6i and newer sunxi SoCs is a two-user mailbox
+  controller containing 8 unidirectional FIFOs. An interrupt is raised for
+  received messages, but software must poll to know when a transmitted message
+  has been acknowledged by the remote user. Each FIFO can hold four 32-bit
+  messages; when a FIFO is full, clients must wait before more transmissions.
+
+  Refer to ./mailbox.txt for generic information about mailbox device-tree
+  bindings.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - allwinner,sun8i-a83t-msgbox
+              - allwinner,sun8i-h3-msgbox
+              - allwinner,sun9i-a80-msgbox
+              - allwinner,sun50i-a64-msgbox
+              - allwinner,sun50i-h6-msgbox
+          - const: allwinner,sun6i-a31-msgbox
+      - items:
+          - const: allwinner,sun6i-a31-msgbox
+
+  reg:
+    items:
+      - description: MMIO register range
+
+  clocks:
+    maxItems: 1
+    description: bus clock
+
+  resets:
+    maxItems: 1
+    description: bus reset
+
+  interrupts:
+    maxItems: 1
+    description: controller interrupt
+
+  '#mbox-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - resets
+  - interrupts
+  - '#mbox-cells'
+
+examples:
+  - |
+    #include <dt-bindings/clock/sun8i-h3-ccu.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/reset/sun8i-h3-ccu.h>
+
+    msgbox: mailbox@1c17000 {
+            compatible = "allwinner,sun8i-h3-msgbox",
+                         "allwinner,sun6i-a31-msgbox";
+            reg = <0x01c17000 0x1000>;
+            clocks = <&ccu CLK_BUS_MSGBOX>;
+            resets = <&ccu RST_BUS_MSGBOX>;
+            interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+            #mbox-cells = <1>;
+    };
+
+...
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver
  2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
                   ` (2 preceding siblings ...)
  2019-08-20  3:23 ` [PATCH v4 03/10] dt-bindings: mailbox: Add a sunxi message box binding Samuel Holland
@ 2019-08-20  3:23 ` Samuel Holland
  2019-08-20  8:27   ` Maxime Ripard
  2019-08-20 11:18   ` Ondřej Jirman
  2019-08-20  3:23 ` [PATCH v4 05/10] ARM: dts: sunxi: a80: Add msgbox node Samuel Holland
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 29+ messages in thread
From: Samuel Holland @ 2019-08-20  3:23 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick
  Cc: devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Samuel Holland

Allwinner sun8i, sun9i, and sun50i SoCs contain a hardware message box
used for communication between the ARM CPUs and the ARISC management
coprocessor. The hardware contains 8 unidirectional 4-message FIFOs.

Add a driver for it, so it can be used for SCPI or other communication
protocols.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/mailbox/Kconfig        |  10 +
 drivers/mailbox/Makefile       |   2 +
 drivers/mailbox/sunxi-msgbox.c | 323 +++++++++++++++++++++++++++++++++
 3 files changed, 335 insertions(+)
 create mode 100644 drivers/mailbox/sunxi-msgbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ab4eb750bbdd..57d12936175e 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -227,4 +227,14 @@ config ZYNQMP_IPI_MBOX
 	  message to the IPI buffer and will access the IPI control
 	  registers to kick the other processor or enquire status.
 
+config SUNXI_MSGBOX
+	tristate "Allwinner sunxi Message Box"
+	depends on ARCH_SUNXI || COMPILE_TEST
+	default ARCH_SUNXI
+	help
+	  Mailbox implementation for the hardware message box present in
+	  Allwinner sun8i, sun9i, and sun50i SoCs. The hardware message box is
+	  used for communication between the application CPUs and the power
+	  management coprocessor.
+
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index c22fad6f696b..bec2d50b0976 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -48,3 +48,5 @@ obj-$(CONFIG_STM32_IPCC) 	+= stm32-ipcc.o
 obj-$(CONFIG_MTK_CMDQ_MBOX)	+= mtk-cmdq-mailbox.o
 
 obj-$(CONFIG_ZYNQMP_IPI_MBOX)	+= zynqmp-ipi-mailbox.o
+
+obj-$(CONFIG_SUNXI_MSGBOX)	+= sunxi-msgbox.o
diff --git a/drivers/mailbox/sunxi-msgbox.c b/drivers/mailbox/sunxi-msgbox.c
new file mode 100644
index 000000000000..29a5101a5390
--- /dev/null
+++ b/drivers/mailbox/sunxi-msgbox.c
@@ -0,0 +1,323 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (c) 2017-2019 Samuel Holland <samuel@sholland.org>
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+
+#define NUM_CHANS		8
+
+#define CTRL_REG(n)		(0x0000 + 0x4 * ((n) / 4))
+#define CTRL_RX(n)		BIT(0 + 8 * ((n) % 4))
+#define CTRL_TX(n)		BIT(4 + 8 * ((n) % 4))
+
+#define REMOTE_IRQ_EN_REG	0x0040
+#define REMOTE_IRQ_STAT_REG	0x0050
+#define LOCAL_IRQ_EN_REG	0x0060
+#define LOCAL_IRQ_STAT_REG	0x0070
+
+#define RX_IRQ(n)		BIT(0 + 2 * (n))
+#define RX_IRQ_MASK		0x5555
+#define TX_IRQ(n)		BIT(1 + 2 * (n))
+#define TX_IRQ_MASK		0xaaaa
+
+#define FIFO_STAT_REG(n)	(0x0100 + 0x4 * (n))
+#define FIFO_STAT_MASK		GENMASK(0, 0)
+
+#define MSG_STAT_REG(n)		(0x0140 + 0x4 * (n))
+#define MSG_STAT_MASK		GENMASK(2, 0)
+
+#define MSG_DATA_REG(n)		(0x0180 + 0x4 * (n))
+
+#define mbox_dbg(mbox, ...)	dev_dbg((mbox)->controller.dev, __VA_ARGS__)
+
+struct sunxi_msgbox {
+	struct mbox_controller controller;
+	struct clk *clk;
+	spinlock_t lock;
+	void __iomem *regs;
+};
+
+static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan);
+static bool sunxi_msgbox_peek_data(struct mbox_chan *chan);
+
+static inline int channel_number(struct mbox_chan *chan)
+{
+	return chan - chan->mbox->chans;
+}
+
+static inline struct sunxi_msgbox *channel_to_msgbox(struct mbox_chan *chan)
+{
+	return chan->con_priv;
+}
+
+static irqreturn_t sunxi_msgbox_irq(int irq, void *dev_id)
+{
+	struct sunxi_msgbox *mbox = dev_id;
+	uint32_t status;
+	int n;
+
+	/* Only examine channels that are currently enabled. */
+	status = readl(mbox->regs + LOCAL_IRQ_EN_REG) &
+		 readl(mbox->regs + LOCAL_IRQ_STAT_REG);
+
+	if (!(status & RX_IRQ_MASK))
+		return IRQ_NONE;
+
+	for (n = 0; n < NUM_CHANS; ++n) {
+		struct mbox_chan *chan = &mbox->controller.chans[n];
+
+		if (!(status & RX_IRQ(n)))
+			continue;
+
+		while (sunxi_msgbox_peek_data(chan)) {
+			uint32_t msg = readl(mbox->regs + MSG_DATA_REG(n));
+
+			mbox_dbg(mbox, "Channel %d received 0x%08x\n", n, msg);
+			mbox_chan_received_data(chan, &msg);
+		}
+
+		/* The IRQ can be cleared only once the FIFO is empty. */
+		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int sunxi_msgbox_send_data(struct mbox_chan *chan, void *data)
+{
+	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
+	int n = channel_number(chan);
+	uint32_t msg = *(uint32_t *)data;
+
+	/* Using a channel backwards gets the hardware into a bad state. */
+	if (WARN_ON_ONCE(!(readl(mbox->regs + CTRL_REG(n)) & CTRL_TX(n))))
+		return 0;
+
+	/* We cannot post a new message if the FIFO is full. */
+	if (readl(mbox->regs + FIFO_STAT_REG(n)) & FIFO_STAT_MASK) {
+		mbox_dbg(mbox, "Channel %d busy sending 0x%08x\n", n, msg);
+		return -EBUSY;
+	}
+
+	writel(msg, mbox->regs + MSG_DATA_REG(n));
+	mbox_dbg(mbox, "Channel %d sent 0x%08x\n", n, msg);
+
+	return 0;
+}
+
+static int sunxi_msgbox_startup(struct mbox_chan *chan)
+{
+	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
+	int n = channel_number(chan);
+
+	/* The coprocessor is responsible for setting channel directions. */
+	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
+		/* Flush the receive FIFO. */
+		while (sunxi_msgbox_peek_data(chan))
+			readl(mbox->regs + MSG_DATA_REG(n));
+		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
+
+		/* Enable the receive IRQ. */
+		spin_lock(&mbox->lock);
+		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) | RX_IRQ(n),
+		       mbox->regs + LOCAL_IRQ_EN_REG);
+		spin_unlock(&mbox->lock);
+	}
+
+	mbox_dbg(mbox, "Channel %d startup complete\n", n);
+
+	return 0;
+}
+
+static void sunxi_msgbox_shutdown(struct mbox_chan *chan)
+{
+	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
+	int n = channel_number(chan);
+
+	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
+		/* Disable the receive IRQ. */
+		spin_lock(&mbox->lock);
+		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) & ~RX_IRQ(n),
+		       mbox->regs + LOCAL_IRQ_EN_REG);
+		spin_unlock(&mbox->lock);
+
+		/* Attempt to flush the FIFO until the IRQ is cleared. */
+		do {
+			while (sunxi_msgbox_peek_data(chan))
+				readl(mbox->regs + MSG_DATA_REG(n));
+			writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
+		} while (readl(mbox->regs + LOCAL_IRQ_STAT_REG) & RX_IRQ(n));
+	}
+
+	mbox_dbg(mbox, "Channel %d shutdown complete\n", n);
+}
+
+static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan)
+{
+	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
+	int n = channel_number(chan);
+
+	/*
+	 * The hardware allows snooping on the remote user's IRQ statuses.
+	 * We consider a message to be acknowledged only once the receive IRQ
+	 * for that channel is cleared. Since the receive IRQ for a channel
+	 * cannot be cleared until the FIFO for that channel is empty, this
+	 * ensures that the message has actually been read. It also gives the
+	 * recipient an opportunity to perform minimal processing before
+	 * acknowledging the message.
+	 */
+	return !(readl(mbox->regs + REMOTE_IRQ_STAT_REG) & RX_IRQ(n));
+}
+
+static bool sunxi_msgbox_peek_data(struct mbox_chan *chan)
+{
+	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
+	int n = channel_number(chan);
+
+	return readl(mbox->regs + MSG_STAT_REG(n)) & MSG_STAT_MASK;
+}
+
+static const struct mbox_chan_ops sunxi_msgbox_chan_ops = {
+	.send_data    = sunxi_msgbox_send_data,
+	.startup      = sunxi_msgbox_startup,
+	.shutdown     = sunxi_msgbox_shutdown,
+	.last_tx_done = sunxi_msgbox_last_tx_done,
+	.peek_data    = sunxi_msgbox_peek_data,
+};
+
+static int sunxi_msgbox_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mbox_chan *chans;
+	struct reset_control *reset;
+	struct resource *res;
+	struct sunxi_msgbox *mbox;
+	int i, ret;
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+
+	chans = devm_kcalloc(dev, NUM_CHANS, sizeof(*chans), GFP_KERNEL);
+	if (!chans)
+		return -ENOMEM;
+
+	for (i = 0; i < NUM_CHANS; ++i)
+		chans[i].con_priv = mbox;
+
+	mbox->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(mbox->clk)) {
+		ret = PTR_ERR(mbox->clk);
+		dev_err(dev, "Failed to get clock: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(mbox->clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable clock: %d\n", ret);
+		return ret;
+	}
+
+	reset = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(reset)) {
+		ret = PTR_ERR(reset);
+		dev_err(dev, "Failed to get reset control: %d\n", ret);
+		goto err_disable_unprepare;
+	}
+
+	ret = reset_control_deassert(reset);
+	if (ret) {
+		dev_err(dev, "Failed to deassert reset: %d\n", ret);
+		goto err_disable_unprepare;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		ret = -ENODEV;
+		goto err_disable_unprepare;
+	}
+
+	mbox->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mbox->regs)) {
+		ret = PTR_ERR(mbox->regs);
+		dev_err(dev, "Failed to map MMIO resource: %d\n", ret);
+		goto err_disable_unprepare;
+	}
+
+	/* Disable all IRQs for this end of the msgbox. */
+	writel(0, mbox->regs + LOCAL_IRQ_EN_REG);
+
+	ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
+			       sunxi_msgbox_irq, 0, dev_name(dev), mbox);
+	if (ret) {
+		dev_err(dev, "Failed to register IRQ handler: %d\n", ret);
+		goto err_disable_unprepare;
+	}
+
+	mbox->controller.dev           = dev;
+	mbox->controller.ops           = &sunxi_msgbox_chan_ops;
+	mbox->controller.chans         = chans;
+	mbox->controller.num_chans     = NUM_CHANS;
+	mbox->controller.txdone_irq    = false;
+	mbox->controller.txdone_poll   = true;
+	mbox->controller.txpoll_period = 5;
+
+	spin_lock_init(&mbox->lock);
+	platform_set_drvdata(pdev, mbox);
+
+	ret = mbox_controller_register(&mbox->controller);
+	if (ret) {
+		dev_err(dev, "Failed to register controller: %d\n", ret);
+		goto err_disable_unprepare;
+	}
+
+	return 0;
+
+err_disable_unprepare:
+	clk_disable_unprepare(mbox->clk);
+
+	return ret;
+}
+
+static int sunxi_msgbox_remove(struct platform_device *pdev)
+{
+	struct sunxi_msgbox *mbox = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(&mbox->controller);
+	clk_disable_unprepare(mbox->clk);
+
+	return 0;
+}
+
+static const struct of_device_id sunxi_msgbox_of_match[] = {
+	{ .compatible = "allwinner,sun6i-a31-msgbox", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sunxi_msgbox_of_match);
+
+static struct platform_driver sunxi_msgbox_driver = {
+	.driver = {
+		.name = "sunxi-msgbox",
+		.of_match_table = sunxi_msgbox_of_match,
+	},
+	.probe  = sunxi_msgbox_probe,
+	.remove = sunxi_msgbox_remove,
+};
+module_platform_driver(sunxi_msgbox_driver);
+
+MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
+MODULE_DESCRIPTION("Allwinner sunxi Message Box");
+MODULE_LICENSE("GPL v2");
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 05/10] ARM: dts: sunxi: a80: Add msgbox node
  2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
                   ` (3 preceding siblings ...)
  2019-08-20  3:23 ` [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver Samuel Holland
@ 2019-08-20  3:23 ` Samuel Holland
  2019-08-20  8:15   ` Maxime Ripard
  2019-08-20  3:23 ` [PATCH v4 06/10] ARM: dts: sunxi: a83t: " Samuel Holland
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 29+ messages in thread
From: Samuel Holland @ 2019-08-20  3:23 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick
  Cc: devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Samuel Holland

The A80 SoC contains a message box that can be used to send messages and
interrupts back and forth between the ARM application CPUs and the ARISC
coprocessor. Add a device tree node for it.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm/boot/dts/sun9i-a80.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi
index c34d505c7efe..844a265dbd0e 100644
--- a/arch/arm/boot/dts/sun9i-a80.dtsi
+++ b/arch/arm/boot/dts/sun9i-a80.dtsi
@@ -318,6 +318,16 @@
 			};
 		};
 
+		msgbox: mailbox@803000 {
+			compatible = "allwinner,sun9i-a80-msgbox",
+				     "allwinner,sun6i-a31-msgbox";
+			reg = <0x00803000 0x1000>;
+			clocks = <&ccu CLK_BUS_MSGBOX>;
+			resets = <&ccu RST_BUS_MSGBOX>;
+			interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+			#mbox-cells = <1>;
+		};
+
 		gmac: ethernet@830000 {
 			compatible = "allwinner,sun7i-a20-gmac";
 			reg = <0x00830000 0x1054>;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 06/10] ARM: dts: sunxi: a83t: Add msgbox node
  2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
                   ` (4 preceding siblings ...)
  2019-08-20  3:23 ` [PATCH v4 05/10] ARM: dts: sunxi: a80: Add msgbox node Samuel Holland
@ 2019-08-20  3:23 ` Samuel Holland
  2019-08-20  3:23 ` [PATCH v4 07/10] ARM: dts: sunxi: h3/h5: " Samuel Holland
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Samuel Holland @ 2019-08-20  3:23 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick
  Cc: devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Samuel Holland

The A83T SoC contains a message box that can be used to send messages
and interrupts back and forth between the ARM application CPUs and the
ARISC coprocessor. Add a device tree node for it.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 523be6611c50..8871d1aaf7f5 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -583,6 +583,16 @@
 			reg = <0x1c14000 0x400>;
 		};
 
+		msgbox: mailbox@1c17000 {
+			compatible = "allwinner,sun8i-a83t-msgbox",
+				     "allwinner,sun6i-a31-msgbox";
+			reg = <0x01c17000 0x1000>;
+			clocks = <&ccu CLK_BUS_MSGBOX>;
+			resets = <&ccu RST_BUS_MSGBOX>;
+			interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+			#mbox-cells = <1>;
+		};
+
 		usb_otg: usb@1c19000 {
 			compatible = "allwinner,sun8i-a83t-musb",
 				     "allwinner,sun8i-a33-musb";
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 07/10] ARM: dts: sunxi: h3/h5: Add msgbox node
  2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
                   ` (5 preceding siblings ...)
  2019-08-20  3:23 ` [PATCH v4 06/10] ARM: dts: sunxi: a83t: " Samuel Holland
@ 2019-08-20  3:23 ` Samuel Holland
  2019-08-20  3:23 ` [PATCH v4 08/10] arm64: dts: allwinner: a64: " Samuel Holland
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Samuel Holland @ 2019-08-20  3:23 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick
  Cc: devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Samuel Holland

The H3 and H5 SoCs contain a message box that can be used to send
messages and interrupts back and forth between the ARM application CPUs
and the ARISC coprocessor. Add a device tree node for it.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 224e105a994a..f25876a8021a 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -232,6 +232,16 @@
 			reg = <0x1c14000 0x400>;
 		};
 
+		msgbox: mailbox@1c17000 {
+			compatible = "allwinner,sun8i-h3-msgbox",
+				     "allwinner,sun6i-a31-msgbox";
+			reg = <0x01c17000 0x1000>;
+			clocks = <&ccu CLK_BUS_MSGBOX>;
+			resets = <&ccu RST_BUS_MSGBOX>;
+			interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+			#mbox-cells = <1>;
+		};
+
 		usb_otg: usb@1c19000 {
 			compatible = "allwinner,sun8i-h3-musb";
 			reg = <0x01c19000 0x400>;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 08/10] arm64: dts: allwinner: a64: Add msgbox node
  2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
                   ` (6 preceding siblings ...)
  2019-08-20  3:23 ` [PATCH v4 07/10] ARM: dts: sunxi: h3/h5: " Samuel Holland
@ 2019-08-20  3:23 ` Samuel Holland
  2019-08-20  3:23 ` [PATCH v4 09/10] arm64: dts: allwinner: h6: " Samuel Holland
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 29+ messages in thread
From: Samuel Holland @ 2019-08-20  3:23 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick
  Cc: devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Samuel Holland

The A64 SoC contains a message box that can be used to send messages and
interrupts back and forth between the ARM application CPUs and the ARISC
coprocessor. Add a device tree node for it.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index ddb6f11e89df..428f539a091a 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -487,6 +487,16 @@
 			reg = <0x1c14000 0x400>;
 		};
 
+		msgbox: mailbox@1c17000 {
+			compatible = "allwinner,sun50i-a64-msgbox",
+				     "allwinner,sun6i-a31-msgbox";
+			reg = <0x01c17000 0x1000>;
+			clocks = <&ccu CLK_BUS_MSGBOX>;
+			resets = <&ccu RST_BUS_MSGBOX>;
+			interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+			#mbox-cells = <1>;
+		};
+
 		usb_otg: usb@1c19000 {
 			compatible = "allwinner,sun8i-a33-musb";
 			reg = <0x01c19000 0x0400>;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 09/10] arm64: dts: allwinner: h6: Add msgbox node
  2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
                   ` (7 preceding siblings ...)
  2019-08-20  3:23 ` [PATCH v4 08/10] arm64: dts: allwinner: a64: " Samuel Holland
@ 2019-08-20  3:23 ` Samuel Holland
  2019-08-20  3:23 ` [PATCH v4 10/10] [DO NOT MERGE] drivers: firmware: msgbox demo Samuel Holland
  2019-09-09  3:22 ` [PATCH v4 00/10] Allwinner sunxi message box support Ondřej Jirman
  10 siblings, 0 replies; 29+ messages in thread
From: Samuel Holland @ 2019-08-20  3:23 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick
  Cc: devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Samuel Holland

The H6 SoC contains a message box that can be used to send messages and
interrupts back and forth between the ARM application CPUs and the ARISC
coprocessor. Add a device tree node for it.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 67b732e34091..2ff6a47e3cbf 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -215,6 +215,16 @@
 			#dma-cells = <1>;
 		};
 
+		msgbox: mailbox@3003000 {
+			compatible = "allwinner,sun50i-h6-msgbox",
+				     "allwinner,sun6i-a31-msgbox";
+			reg = <0x03003000 0x1000>;
+			clocks = <&ccu CLK_BUS_MSGBOX>;
+			resets = <&ccu RST_BUS_MSGBOX>;
+			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
+			#mbox-cells = <1>;
+		};
+
 		sid: efuse@3006000 {
 			compatible = "allwinner,sun50i-h6-sid";
 			reg = <0x03006000 0x400>;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 10/10] [DO NOT MERGE] drivers: firmware: msgbox demo
  2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
                   ` (8 preceding siblings ...)
  2019-08-20  3:23 ` [PATCH v4 09/10] arm64: dts: allwinner: h6: " Samuel Holland
@ 2019-08-20  3:23 ` Samuel Holland
  2019-09-09  3:22 ` [PATCH v4 00/10] Allwinner sunxi message box support Ondřej Jirman
  10 siblings, 0 replies; 29+ messages in thread
From: Samuel Holland @ 2019-08-20  3:23 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick
  Cc: devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Samuel Holland

This driver provides a trivial mailbox client that can be used with the
mailbox-demo branch of https://github.com/crust-firmware/crust for
verifying the functionality of the sunxi-msgbox driver.

This is not a "real" driver, nor a "real" firmware protocol. This driver
is not intended to be merged. It is provided only as an example that
won't interfere with any other hardware.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  24 ++
 arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  24 ++
 drivers/firmware/Kconfig                      |   6 +
 drivers/firmware/Makefile                     |   1 +
 drivers/firmware/sunxi_msgbox_demo.c          | 310 ++++++++++++++++++
 5 files changed, 365 insertions(+)
 create mode 100644 drivers/firmware/sunxi_msgbox_demo.c

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 428f539a091a..78315d5512db 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -121,6 +121,30 @@
 		};
 	};
 
+	demo_0 {
+		compatible = "allwinner,sunxi-msgbox-demo";
+		mboxes = <&msgbox 0>, <&msgbox 1>;
+		mbox-names = "tx", "rx";
+	};
+
+	demo_1 {
+		compatible = "allwinner,sunxi-msgbox-demo";
+		mboxes = <&msgbox 2>, <&msgbox 3>;
+		mbox-names = "tx", "rx";
+	};
+
+	demo_2 {
+		compatible = "allwinner,sunxi-msgbox-demo";
+		mboxes = <&msgbox 4>, <&msgbox 5>;
+		mbox-names = "tx", "rx";
+	};
+
+	demo_3 {
+		compatible = "allwinner,sunxi-msgbox-demo";
+		mboxes = <&msgbox 6>, <&msgbox 7>;
+		mbox-names = "tx", "rx";
+	};
+
 	de: display-engine {
 		compatible = "allwinner,sun50i-a64-display-engine";
 		allwinner,pipelines = <&mixer0>,
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
index f002a496d7cb..5a2d85b7e0a1 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
@@ -76,6 +76,30 @@
 		};
 	};
 
+	demo_0 {
+		compatible = "allwinner,sunxi-msgbox-demo";
+		mboxes = <&msgbox 0>, <&msgbox 1>;
+		mbox-names = "tx", "rx";
+	};
+
+	demo_1 {
+		compatible = "allwinner,sunxi-msgbox-demo";
+		mboxes = <&msgbox 2>, <&msgbox 3>;
+		mbox-names = "tx", "rx";
+	};
+
+	demo_2 {
+		compatible = "allwinner,sunxi-msgbox-demo";
+		mboxes = <&msgbox 4>, <&msgbox 5>;
+		mbox-names = "tx", "rx";
+	};
+
+	demo_3 {
+		compatible = "allwinner,sunxi-msgbox-demo";
+		mboxes = <&msgbox 6>, <&msgbox 7>;
+		mbox-names = "tx", "rx";
+	};
+
 	psci {
 		compatible = "arm,psci-0.2";
 		method = "smc";
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index ba8d3d0ef32c..e0f8f3c856c1 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -240,6 +240,12 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
 
 	  Say Y here to enable "download mode" by default.
 
+config SUNXI_MSGBOX_DEMO
+	tristate "sunxi msgbox demo"
+	depends on MAILBOX
+	help
+	  Demo client for demo firmware to use in mailbox driver validation.
+
 config TI_SCI_PROTOCOL
 	tristate "TI System Control Interface (TISCI) Message Protocol"
 	depends on TI_MESSAGE_MANAGER
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 3fa0b34eb72f..6f8e17a854b6 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_QCOM_SCM)		+= qcom_scm.o
 obj-$(CONFIG_QCOM_SCM_64)	+= qcom_scm-64.o
 obj-$(CONFIG_QCOM_SCM_32)	+= qcom_scm-32.o
 CFLAGS_qcom_scm-32.o :=$(call as-instr,.arch armv7-a\n.arch_extension sec,-DREQUIRES_SEC=1) -march=armv7-a
+obj-$(CONFIG_SUNXI_MSGBOX_DEMO)	+= sunxi_msgbox_demo.o
 obj-$(CONFIG_TI_SCI_PROTOCOL)	+= ti_sci.o
 obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
 
diff --git a/drivers/firmware/sunxi_msgbox_demo.c b/drivers/firmware/sunxi_msgbox_demo.c
new file mode 100644
index 000000000000..9431b1ef1841
--- /dev/null
+++ b/drivers/firmware/sunxi_msgbox_demo.c
@@ -0,0 +1,310 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (c) 2018-2019 Samuel Holland <samuel@sholland.org>
+
+#include <linux/completion.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/random.h>
+
+enum {
+	OP_MAGIC,
+	OP_VERSION,
+	OP_LOOPBACK,
+	OP_LOOPBACK_INVERTED,
+	OP_TIME_SECONDS,
+	OP_TIME_TICKS,
+	OP_DELAY_MICROS,
+	OP_DELAY_MILLIS,
+	OP_ADDR_SET_LO,
+	OP_ADDR_SET_HI,
+	OP_ADDR_READ,
+	OP_ADDR_WRITE,
+	OP_INVALID_1,
+	OP_INVALID_2,
+	OP_RESET = 16,
+};
+
+struct msgbox_demo {
+	struct mbox_chan *rx_chan;
+	struct mbox_chan *tx_chan;
+	struct mbox_client cl;
+	struct completion completion;
+	uint32_t request;
+	uint32_t response;
+	uint32_t address;
+	uint32_t value;
+};
+
+static void msgbox_demo_rx(struct mbox_client *cl, void *msg)
+{
+	struct msgbox_demo *demo = container_of(cl, struct msgbox_demo, cl);
+
+	demo->response = *(uint32_t *)msg;
+	complete(&demo->completion);
+}
+
+static int msgbox_demo_tx(struct msgbox_demo *demo, uint32_t request)
+{
+	unsigned long timeout = msecs_to_jiffies(10);
+	int ret;
+
+	demo->request  = request;
+	demo->response = 0;
+	reinit_completion(&demo->completion);
+
+	ret = mbox_send_message(demo->tx_chan, &demo->request);
+	if (ret < 0) {
+		dev_err(demo->cl.dev, "Failed to send request: %d\n", ret);
+		return ret;
+	}
+
+	if (wait_for_completion_timeout(&demo->completion, timeout))
+		return 0;
+
+	return -ETIMEDOUT;
+}
+
+static void msgbox_demo_do_operation(struct msgbox_demo *demo, uint16_t op)
+{
+	struct device *dev = demo->cl.dev;
+	uint16_t data = 0;
+	uint32_t resp = 0;
+	int exp = 0;
+	int ret;
+
+	switch (op) {
+	case OP_MAGIC:
+		resp = 0x1a2a3a4a;
+		break;
+	case OP_LOOPBACK:
+		data = get_random_u32();
+		resp = data;
+		break;
+	case OP_LOOPBACK_INVERTED:
+		data = get_random_u32();
+		resp = ~data;
+		break;
+	case OP_DELAY_MICROS:
+		data = 25000;
+		exp  = -ETIMEDOUT;
+		break;
+	case OP_DELAY_MILLIS:
+		data = 500;
+		exp  = -ETIMEDOUT;
+		break;
+	case OP_ADDR_SET_LO:
+		data = demo->address & 0xffff;
+		resp = demo->address;
+		break;
+	case OP_ADDR_SET_HI:
+		data = demo->address >> 16;
+		break;
+	case OP_ADDR_WRITE:
+		data = demo->value;
+		resp = demo->value;
+		break;
+	case OP_INVALID_1:
+	case OP_INVALID_2:
+		resp = -1U;
+		break;
+	case OP_RESET:
+		exp  = -ETIMEDOUT;
+		break;
+	}
+
+	dev_info(demo->cl.dev, "Sending opcode %d, data 0x%08x\n", op, data);
+	ret = msgbox_demo_tx(demo, op << 16 | data);
+
+	if (ret) {
+		/* Nothing was received. */
+		if (exp)
+			dev_info(dev, "No response received, as expected\n");
+		else
+			dev_err(dev, "Timeout receiving response\n");
+		return;
+	}
+
+	/* Something was received. */
+	if (exp)
+		dev_err(dev, "Unexpected response 0x%08x\n", demo->response);
+	else if (!resp)
+		dev_info(dev, "Received response 0x%08x\n", demo->response);
+	else if (demo->response == resp)
+		dev_info(dev, "Good response 0x%08x\n", resp);
+	else
+		dev_err(dev, "Expected 0x%08x, received 0x%08x\n",
+			     resp, demo->response);
+}
+
+ssize_t demo_address_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct msgbox_demo *demo = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%08x\n", demo->address);
+}
+
+static ssize_t demo_address_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct msgbox_demo *demo = dev_get_drvdata(dev);
+	uint32_t val;
+
+	if (sscanf(buf, "%x", &val)) {
+		demo->address = val;
+		msgbox_demo_do_operation(demo, OP_ADDR_SET_HI);
+		msgbox_demo_do_operation(demo, OP_ADDR_SET_LO);
+		return count;
+	}
+
+	return 0;
+}
+
+ssize_t demo_value_show(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct msgbox_demo *demo = dev_get_drvdata(dev);
+
+	msgbox_demo_do_operation(demo, OP_ADDR_READ);
+	demo->value = demo->response;
+
+	return sprintf(buf, "%08x\n", demo->value);
+}
+
+static ssize_t demo_value_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct msgbox_demo *demo = dev_get_drvdata(dev);
+	int16_t val;
+
+	if (sscanf(buf, "%hx", &val)) {
+		demo->value = (int32_t)val;
+		msgbox_demo_do_operation(demo, OP_ADDR_WRITE);
+		return count;
+	}
+
+	return 0;
+}
+
+static ssize_t demo_operation_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	struct msgbox_demo *demo = dev_get_drvdata(dev);
+	uint16_t val;
+
+	if (sscanf(buf, "%hu", &val)) {
+		msgbox_demo_do_operation(demo, val);
+		return count;
+	}
+
+	return 0;
+}
+
+static DEVICE_ATTR(demo_address,   0644, demo_address_show, demo_address_store);
+static DEVICE_ATTR(demo_value,     0644, demo_value_show,   demo_value_store);
+static DEVICE_ATTR(demo_operation, 0200, NULL,              demo_operation_store);
+
+static int msgbox_demo_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_attribute *attr;
+	struct msgbox_demo *demo;
+	int ret;
+
+	demo = devm_kzalloc(dev, sizeof(*demo), GFP_KERNEL);
+	if (!demo)
+		return -ENOMEM;
+
+	demo->cl.dev         = dev;
+	demo->cl.rx_callback = msgbox_demo_rx;
+
+	if (of_get_property(dev->of_node, "mbox-names", NULL)) {
+		demo->rx_chan = mbox_request_channel_byname(&demo->cl, "rx");
+		if (IS_ERR(demo->rx_chan)) {
+			ret = PTR_ERR(demo->rx_chan);
+			dev_err(dev, "Failed to request rx mailbox channel\n");
+			goto err;
+		}
+		demo->tx_chan = mbox_request_channel_byname(&demo->cl, "tx");
+		if (IS_ERR(demo->tx_chan)) {
+			ret = PTR_ERR(demo->tx_chan);
+			dev_err(dev, "Failed to request tx mailbox channel\n");
+			goto err_free_rx_chan;
+		}
+	} else {
+		demo->rx_chan = mbox_request_channel(&demo->cl, 0);
+		demo->tx_chan = demo->rx_chan;
+		if (IS_ERR(demo->tx_chan)) {
+			ret = PTR_ERR(demo->tx_chan);
+			dev_err(dev, "Failed to request mailbox channel\n");
+			goto err;
+		}
+	}
+
+	attr = &dev_attr_demo_address;
+	ret = device_create_file(dev, attr);
+	if (ret)
+		goto err_creating_files;
+	attr = &dev_attr_demo_value;
+	ret = device_create_file(dev, attr);
+	if (ret)
+		goto err_creating_files;
+	attr = &dev_attr_demo_operation;
+	ret = device_create_file(dev, attr);
+	if (ret)
+		goto err_creating_files;
+
+	init_completion(&demo->completion);
+
+	platform_set_drvdata(pdev, demo);
+
+	msgbox_demo_do_operation(demo, OP_VERSION);
+
+	return 0;
+
+err_creating_files:
+	dev_err(dev, "Failed to create sysfs attribute %s: %d\n",
+		attr->attr.name, ret);
+	if (demo->tx_chan != demo->rx_chan)
+		mbox_free_channel(demo->tx_chan);
+err_free_rx_chan:
+	mbox_free_channel(demo->rx_chan);
+err:
+	return ret;
+}
+
+static int msgbox_demo_remove(struct platform_device *pdev)
+{
+	struct msgbox_demo *demo = platform_get_drvdata(pdev);
+
+	if (demo->tx_chan != demo->rx_chan)
+		mbox_free_channel(demo->tx_chan);
+	mbox_free_channel(demo->rx_chan);
+
+	return 0;
+}
+
+static const struct of_device_id msgbox_demo_of_match[] = {
+	{ .compatible = "allwinner,sunxi-msgbox-demo" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, msgbox_demo_of_match);
+
+static struct platform_driver msgbox_demo_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = msgbox_demo_of_match,
+	},
+	.probe  = msgbox_demo_probe,
+	.remove = msgbox_demo_remove,
+};
+module_platform_driver(msgbox_demo_driver);
+
+MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
+MODULE_DESCRIPTION("sunxi msgbox demo");
+MODULE_LICENSE("GPL v2");
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 02/10] clk: sunxi-ng: Mark AR100 clocks as critical
  2019-08-20  3:23 ` [PATCH v4 02/10] clk: sunxi-ng: Mark AR100 " Samuel Holland
@ 2019-08-20  7:11   ` Maxime Ripard
  2019-08-20 13:02     ` Samuel Holland
  0 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-08-20  7:11 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, Corentin Labbe, Vasily Khoruzhick,
	devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi

[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]

Hi,

On Mon, Aug 19, 2019 at 10:23:03PM -0500, Samuel Holland wrote:
> On sun8i, sun9i, and sun50i SoCs, system suspend/resume support requires
> firmware running on the AR100 coprocessor (the "SCP"). Such firmware can
> provide additional features, such as thermal monitoring and poweron/off
> support for boards without a PMIC.
>
> Since the AR100 may be running critical firmware, even if Linux does not
> know about it or directly interact with it (all requests may go through
> an intermediary interface such as PSCI), Linux must not turn off its
> clock.
>
> At this time, such power management firmware only exists for the A64 and
> H5 SoCs.  However, it makes sense to take care of all CCU drivers now
> for consistency, and to ease the transition in the future once firmware
> is ported to the other SoCs.
>
> Leaving the clock running is safe even if no firmware is present, since
> the AR100 stays in reset by default. In most cases, the AR100 clock is
> kept enabled by Linux anyway, since it is the parent of all APB0 bus
> peripherals. This change only prevents Linux from turning off the AR100
> clock in the rare case that no peripherals are in use.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>

So I'm not really sure where you want to go with this.

That clock is only useful where you're having a firmware running on
the AR100, and that firmware would have a device tree node of its own,
where we could list the clocks needed for the firmware to keep
running, if it ever runs. If the driver has not been compiled in /
loaded, then we don't care either.

But more fundamentally, if we're going to use SCPI, then those clocks
will not be handled by that driver anyway, but by the firmware, right?

So I'm not really sure that we should do it statically this way, and
that we should do it at all.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 03/10] dt-bindings: mailbox: Add a sunxi message box binding
  2019-08-20  3:23 ` [PATCH v4 03/10] dt-bindings: mailbox: Add a sunxi message box binding Samuel Holland
@ 2019-08-20  7:14   ` Maxime Ripard
  2019-08-20 13:04     ` Samuel Holland
  0 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-08-20  7:14 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, Corentin Labbe, Vasily Khoruzhick,
	devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Rob Herring

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

Hi,

On Mon, Aug 19, 2019 at 10:23:04PM -0500, Samuel Holland wrote:
> This mailbox hardware is present in Allwinner sun8i, sun9i, and sun50i
> SoCs. Add a device tree binding for it.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  .../mailbox/allwinner,sunxi-msgbox.yaml       | 79 +++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml

So we merged a bunch of schemas already, with the convention that the
name was the first compatible to use that binding.

That would be allwinner,sun6i-a31-msgbox.yaml in that case

Thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 05/10] ARM: dts: sunxi: a80: Add msgbox node
  2019-08-20  3:23 ` [PATCH v4 05/10] ARM: dts: sunxi: a80: Add msgbox node Samuel Holland
@ 2019-08-20  8:15   ` Maxime Ripard
  2019-08-20 13:17     ` Samuel Holland
  0 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-08-20  8:15 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, Corentin Labbe, Vasily Khoruzhick,
	devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi

[-- Attachment #1: Type: text/plain, Size: 558 bytes --]

Hi,

On Mon, Aug 19, 2019 at 10:23:06PM -0500, Samuel Holland wrote:
> The A80 SoC contains a message box that can be used to send messages and
> interrupts back and forth between the ARM application CPUs and the ARISC
> coprocessor. Add a device tree node for it.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>

I think you mentionned that crust has been tested only on the A64 and
the H3/H5, did you test the mailbox on those other SoCs as well?

Thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver
  2019-08-20  3:23 ` [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver Samuel Holland
@ 2019-08-20  8:27   ` Maxime Ripard
  2019-08-20 11:18   ` Ondřej Jirman
  1 sibling, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2019-08-20  8:27 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, Corentin Labbe, Vasily Khoruzhick,
	devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi

[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]

Hi,

On Mon, Aug 19, 2019 at 10:23:05PM -0500, Samuel Holland wrote:
> Allwinner sun8i, sun9i, and sun50i SoCs contain a hardware message box
> used for communication between the ARM CPUs and the ARISC management
> coprocessor. The hardware contains 8 unidirectional 4-message FIFOs.
>
> Add a driver for it, so it can be used for SCPI or other communication
> protocols.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  drivers/mailbox/Kconfig        |  10 +
>  drivers/mailbox/Makefile       |   2 +
>  drivers/mailbox/sunxi-msgbox.c | 323 +++++++++++++++++++++++++++++++++
>  3 files changed, 335 insertions(+)
>  create mode 100644 drivers/mailbox/sunxi-msgbox.c

It's pretty much the same remark than for the name of the binding
file, but sunxi in itself is pretty confusing, it covers a range of
SoCs going from armv5 to armv8, some with a single CPU and some with
more, and some with an OpenRISC core and some without.

It would be less confusing (albeit not perfect) to use sun6i there,
the family that IP was first introduced in.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver
  2019-08-20  3:23 ` [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver Samuel Holland
  2019-08-20  8:27   ` Maxime Ripard
@ 2019-08-20 11:18   ` Ondřej Jirman
  2019-08-20 13:07     ` Samuel Holland
  1 sibling, 1 reply; 29+ messages in thread
From: Ondřej Jirman @ 2019-08-20 11:18 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick, devicetree, linux-kernel, linux-sunxi,
	linux-clk, linux-arm-kernel

Hi Samuel,

On Mon, Aug 19, 2019 at 10:23:05PM -0500, Samuel Holland wrote:
> Allwinner sun8i, sun9i, and sun50i SoCs contain a hardware message box
> used for communication between the ARM CPUs and the ARISC management
> coprocessor. The hardware contains 8 unidirectional 4-message FIFOs.
> 
> Add a driver for it, so it can be used for SCPI or other communication
> protocols.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  drivers/mailbox/Kconfig        |  10 +
>  drivers/mailbox/Makefile       |   2 +
>  drivers/mailbox/sunxi-msgbox.c | 323 +++++++++++++++++++++++++++++++++
>  3 files changed, 335 insertions(+)
>  create mode 100644 drivers/mailbox/sunxi-msgbox.c
> 
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index ab4eb750bbdd..57d12936175e 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -227,4 +227,14 @@ config ZYNQMP_IPI_MBOX
>  	  message to the IPI buffer and will access the IPI control
>  	  registers to kick the other processor or enquire status.
>  
> +config SUNXI_MSGBOX
> +	tristate "Allwinner sunxi Message Box"
> +	depends on ARCH_SUNXI || COMPILE_TEST
> +	default ARCH_SUNXI
> +	help
> +	  Mailbox implementation for the hardware message box present in
> +	  Allwinner sun8i, sun9i, and sun50i SoCs. The hardware message box is
> +	  used for communication between the application CPUs and the power
> +	  management coprocessor.
> +
>  endif
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index c22fad6f696b..bec2d50b0976 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -48,3 +48,5 @@ obj-$(CONFIG_STM32_IPCC) 	+= stm32-ipcc.o
>  obj-$(CONFIG_MTK_CMDQ_MBOX)	+= mtk-cmdq-mailbox.o
>  
>  obj-$(CONFIG_ZYNQMP_IPI_MBOX)	+= zynqmp-ipi-mailbox.o
> +
> +obj-$(CONFIG_SUNXI_MSGBOX)	+= sunxi-msgbox.o
> diff --git a/drivers/mailbox/sunxi-msgbox.c b/drivers/mailbox/sunxi-msgbox.c
> new file mode 100644
> index 000000000000..29a5101a5390
> --- /dev/null
> +++ b/drivers/mailbox/sunxi-msgbox.c
> @@ -0,0 +1,323 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (c) 2017-2019 Samuel Holland <samuel@sholland.org>
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +
> +#define NUM_CHANS		8
> +
> +#define CTRL_REG(n)		(0x0000 + 0x4 * ((n) / 4))
> +#define CTRL_RX(n)		BIT(0 + 8 * ((n) % 4))
> +#define CTRL_TX(n)		BIT(4 + 8 * ((n) % 4))
> +
> +#define REMOTE_IRQ_EN_REG	0x0040
> +#define REMOTE_IRQ_STAT_REG	0x0050
> +#define LOCAL_IRQ_EN_REG	0x0060
> +#define LOCAL_IRQ_STAT_REG	0x0070
> +
> +#define RX_IRQ(n)		BIT(0 + 2 * (n))
> +#define RX_IRQ_MASK		0x5555
> +#define TX_IRQ(n)		BIT(1 + 2 * (n))
> +#define TX_IRQ_MASK		0xaaaa
> +
> +#define FIFO_STAT_REG(n)	(0x0100 + 0x4 * (n))
> +#define FIFO_STAT_MASK		GENMASK(0, 0)
> +
> +#define MSG_STAT_REG(n)		(0x0140 + 0x4 * (n))
> +#define MSG_STAT_MASK		GENMASK(2, 0)
> +
> +#define MSG_DATA_REG(n)		(0x0180 + 0x4 * (n))
> +
> +#define mbox_dbg(mbox, ...)	dev_dbg((mbox)->controller.dev, __VA_ARGS__)
> +
> +struct sunxi_msgbox {
> +	struct mbox_controller controller;
> +	struct clk *clk;
> +	spinlock_t lock;
> +	void __iomem *regs;
> +};
> +
> +static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan);
> +static bool sunxi_msgbox_peek_data(struct mbox_chan *chan);
> +
> +static inline int channel_number(struct mbox_chan *chan)
> +{
> +	return chan - chan->mbox->chans;
> +}
> +
> +static inline struct sunxi_msgbox *channel_to_msgbox(struct mbox_chan *chan)
> +{
> +	return chan->con_priv;
> +}
> +
> +static irqreturn_t sunxi_msgbox_irq(int irq, void *dev_id)
> +{
> +	struct sunxi_msgbox *mbox = dev_id;
> +	uint32_t status;
> +	int n;
> +
> +	/* Only examine channels that are currently enabled. */
> +	status = readl(mbox->regs + LOCAL_IRQ_EN_REG) &
> +		 readl(mbox->regs + LOCAL_IRQ_STAT_REG);
> +
> +	if (!(status & RX_IRQ_MASK))
> +		return IRQ_NONE;
> +
> +	for (n = 0; n < NUM_CHANS; ++n) {
> +		struct mbox_chan *chan = &mbox->controller.chans[n];
> +
> +		if (!(status & RX_IRQ(n)))
> +			continue;
> +
> +		while (sunxi_msgbox_peek_data(chan)) {
> +			uint32_t msg = readl(mbox->regs + MSG_DATA_REG(n));
> +
> +			mbox_dbg(mbox, "Channel %d received 0x%08x\n", n, msg);
> +			mbox_chan_received_data(chan, &msg);
> +		}
> +
> +		/* The IRQ can be cleared only once the FIFO is empty. */
> +		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int sunxi_msgbox_send_data(struct mbox_chan *chan, void *data)
> +{
> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> +	int n = channel_number(chan);
> +	uint32_t msg = *(uint32_t *)data;
> +
> +	/* Using a channel backwards gets the hardware into a bad state. */
> +	if (WARN_ON_ONCE(!(readl(mbox->regs + CTRL_REG(n)) & CTRL_TX(n))))
> +		return 0;
> +
> +	/* We cannot post a new message if the FIFO is full. */
> +	if (readl(mbox->regs + FIFO_STAT_REG(n)) & FIFO_STAT_MASK) {
> +		mbox_dbg(mbox, "Channel %d busy sending 0x%08x\n", n, msg);
> +		return -EBUSY;
> +	}
> +
> +	writel(msg, mbox->regs + MSG_DATA_REG(n));
> +	mbox_dbg(mbox, "Channel %d sent 0x%08x\n", n, msg);
> +
> +	return 0;
> +}
> +
> +static int sunxi_msgbox_startup(struct mbox_chan *chan)
> +{
> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> +	int n = channel_number(chan);
> +
> +	/* The coprocessor is responsible for setting channel directions. */
> +	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
> +		/* Flush the receive FIFO. */
> +		while (sunxi_msgbox_peek_data(chan))
> +			readl(mbox->regs + MSG_DATA_REG(n));
> +		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
> +
> +		/* Enable the receive IRQ. */
> +		spin_lock(&mbox->lock);
> +		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) | RX_IRQ(n),
> +		       mbox->regs + LOCAL_IRQ_EN_REG);
> +		spin_unlock(&mbox->lock);
> +	}
> +
> +	mbox_dbg(mbox, "Channel %d startup complete\n", n);
> +
> +	return 0;
> +}
> +
> +static void sunxi_msgbox_shutdown(struct mbox_chan *chan)
> +{
> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> +	int n = channel_number(chan);
> +
> +	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
> +		/* Disable the receive IRQ. */
> +		spin_lock(&mbox->lock);
> +		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) & ~RX_IRQ(n),
> +		       mbox->regs + LOCAL_IRQ_EN_REG);
> +		spin_unlock(&mbox->lock);
> +
> +		/* Attempt to flush the FIFO until the IRQ is cleared. */
> +		do {
> +			while (sunxi_msgbox_peek_data(chan))
> +				readl(mbox->regs + MSG_DATA_REG(n));
> +			writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
> +		} while (readl(mbox->regs + LOCAL_IRQ_STAT_REG) & RX_IRQ(n));
> +	}
> +
> +	mbox_dbg(mbox, "Channel %d shutdown complete\n", n);
> +}
> +
> +static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan)
> +{
> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> +	int n = channel_number(chan);
> +
> +	/*
> +	 * The hardware allows snooping on the remote user's IRQ statuses.
> +	 * We consider a message to be acknowledged only once the receive IRQ
> +	 * for that channel is cleared. Since the receive IRQ for a channel
> +	 * cannot be cleared until the FIFO for that channel is empty, this
> +	 * ensures that the message has actually been read. It also gives the
> +	 * recipient an opportunity to perform minimal processing before
> +	 * acknowledging the message.
> +	 */
> +	return !(readl(mbox->regs + REMOTE_IRQ_STAT_REG) & RX_IRQ(n));
> +}
> +
> +static bool sunxi_msgbox_peek_data(struct mbox_chan *chan)
> +{
> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> +	int n = channel_number(chan);
> +
> +	return readl(mbox->regs + MSG_STAT_REG(n)) & MSG_STAT_MASK;
> +}
> +
> +static const struct mbox_chan_ops sunxi_msgbox_chan_ops = {
> +	.send_data    = sunxi_msgbox_send_data,
> +	.startup      = sunxi_msgbox_startup,
> +	.shutdown     = sunxi_msgbox_shutdown,
> +	.last_tx_done = sunxi_msgbox_last_tx_done,
> +	.peek_data    = sunxi_msgbox_peek_data,
> +};
> +
> +static int sunxi_msgbox_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mbox_chan *chans;
> +	struct reset_control *reset;
> +	struct resource *res;
> +	struct sunxi_msgbox *mbox;
> +	int i, ret;
> +
> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> +	if (!mbox)
> +		return -ENOMEM;
> +
> +	chans = devm_kcalloc(dev, NUM_CHANS, sizeof(*chans), GFP_KERNEL);
> +	if (!chans)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < NUM_CHANS; ++i)
> +		chans[i].con_priv = mbox;
> +
> +	mbox->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(mbox->clk)) {
> +		ret = PTR_ERR(mbox->clk);
> +		dev_err(dev, "Failed to get clock: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(mbox->clk);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable clock: %d\n", ret);
> +		return ret;
> +	}
> +
> +	reset = devm_reset_control_get(dev, NULL);
> +	if (IS_ERR(reset)) {
> +		ret = PTR_ERR(reset);
> +		dev_err(dev, "Failed to get reset control: %d\n", ret);
> +		goto err_disable_unprepare;
> +	}
> +
> +	ret = reset_control_deassert(reset);
> +	if (ret) {
> +		dev_err(dev, "Failed to deassert reset: %d\n", ret);
> +		goto err_disable_unprepare;
> +	}

You need to assert the reset again from now on, in error paths. devm
will not do that for you.

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		ret = -ENODEV;
> +		goto err_disable_unprepare;
> +	}
> +
> +	mbox->regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(mbox->regs)) {
> +		ret = PTR_ERR(mbox->regs);
> +		dev_err(dev, "Failed to map MMIO resource: %d\n", ret);
> +		goto err_disable_unprepare;
> +	}
> +
> +	/* Disable all IRQs for this end of the msgbox. */
> +	writel(0, mbox->regs + LOCAL_IRQ_EN_REG);
> +
> +	ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
> +			       sunxi_msgbox_irq, 0, dev_name(dev), mbox);
> +	if (ret) {
> +		dev_err(dev, "Failed to register IRQ handler: %d\n", ret);
> +		goto err_disable_unprepare;
> +	}
> +
> +	mbox->controller.dev           = dev;
> +	mbox->controller.ops           = &sunxi_msgbox_chan_ops;
> +	mbox->controller.chans         = chans;
> +	mbox->controller.num_chans     = NUM_CHANS;
> +	mbox->controller.txdone_irq    = false;
> +	mbox->controller.txdone_poll   = true;
> +	mbox->controller.txpoll_period = 5;
> +
> +	spin_lock_init(&mbox->lock);
> +	platform_set_drvdata(pdev, mbox);
> +
> +	ret = mbox_controller_register(&mbox->controller);
> +	if (ret) {
> +		dev_err(dev, "Failed to register controller: %d\n", ret);
> +		goto err_disable_unprepare;
> +	}
> +
> +	return 0;
> +
> +err_disable_unprepare:
> +	clk_disable_unprepare(mbox->clk);
> +
> +	return ret;
> +}
> +
> +static int sunxi_msgbox_remove(struct platform_device *pdev)
> +{
> +	struct sunxi_msgbox *mbox = platform_get_drvdata(pdev);
> +
> +	mbox_controller_unregister(&mbox->controller);
> +	clk_disable_unprepare(mbox->clk);

Also, assert the reset here.

regards,
	o.

> +	return 0;
> +}
> +
> +static const struct of_device_id sunxi_msgbox_of_match[] = {
> +	{ .compatible = "allwinner,sun6i-a31-msgbox", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sunxi_msgbox_of_match);
> +
> +static struct platform_driver sunxi_msgbox_driver = {
> +	.driver = {
> +		.name = "sunxi-msgbox",
> +		.of_match_table = sunxi_msgbox_of_match,
> +	},
> +	.probe  = sunxi_msgbox_probe,
> +	.remove = sunxi_msgbox_remove,
> +};
> +module_platform_driver(sunxi_msgbox_driver);
> +
> +MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
> +MODULE_DESCRIPTION("Allwinner sunxi Message Box");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.21.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 02/10] clk: sunxi-ng: Mark AR100 clocks as critical
  2019-08-20  7:11   ` Maxime Ripard
@ 2019-08-20 13:02     ` Samuel Holland
  2019-08-21 12:24       ` Maxime Ripard
  0 siblings, 1 reply; 29+ messages in thread
From: Samuel Holland @ 2019-08-20 13:02 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, Corentin Labbe, Vasily Khoruzhick,
	devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi

On 8/20/19 2:11 AM, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Aug 19, 2019 at 10:23:03PM -0500, Samuel Holland wrote:
>> On sun8i, sun9i, and sun50i SoCs, system suspend/resume support requires
>> firmware running on the AR100 coprocessor (the "SCP"). Such firmware can
>> provide additional features, such as thermal monitoring and poweron/off
>> support for boards without a PMIC.
>>
>> Since the AR100 may be running critical firmware, even if Linux does not
>> know about it or directly interact with it (all requests may go through
>> an intermediary interface such as PSCI), Linux must not turn off its
>> clock.

This paragraph here is the key. The firmware won't necessarily have a device
tree node, and in the current design it will not, since Linux never communicates
with it directly. All communication goes through ATF via PSCI.

>> At this time, such power management firmware only exists for the A64 and
>> H5 SoCs.  However, it makes sense to take care of all CCU drivers now
>> for consistency, and to ease the transition in the future once firmware
>> is ported to the other SoCs.
>>
>> Leaving the clock running is safe even if no firmware is present, since
>> the AR100 stays in reset by default. In most cases, the AR100 clock is
>> kept enabled by Linux anyway, since it is the parent of all APB0 bus
>> peripherals. This change only prevents Linux from turning off the AR100
>> clock in the rare case that no peripherals are in use.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
> 
> So I'm not really sure where you want to go with this.
> 
> That clock is only useful where you're having a firmware running on
> the AR100, and that firmware would have a device tree node of its own,
> where we could list the clocks needed for the firmware to keep
> running, if it ever runs. If the driver has not been compiled in /
> loaded, then we don't care either.

See above. I don't expect that the firmware would have a device tree node,
because the firmware doesn't need any Linux drivers.

> But more fundamentally, if we're going to use SCPI, then those clocks
> will not be handled by that driver anyway, but by the firmware, right?

In the future, we might use SCPI clocks/sensors/regulators/etc. from Linux, but
that's not the plan at the moment. Given that it's already been two years since
I started this project, I'm trying to limit its scope so I can get at least some
part merged. The first step is to integrate a firmware that provides
suspend/resume functionality only. That firmware does implement SCPI, and if the
top-level Linux SCPI driver worked with multiple mailbox channels, it could
query the firmware's version and fetures. But all of the SCPI commands used for
suspend/resume must go through ATF via PSCI.

> So I'm not really sure that we should do it statically this way, and
> that we should do it at all.

Do you have a better way to model "firmware uses this clock behind the scenes,
so Linux please don't touch it"? It's unfortunate that we have Linux and
firmware fighting over the R_CCU, but since we didn't have firmware (e.g. SCPI
clocks) in the beginning, it's where we are today.

The AR100 clock doesn't actually have a gate, and it generally has dependencies
like R_INTC in use. So as I mentioned in the commit message, the clock will
normally be on anyway. The goal was to model the fact that there are users of
this clock that Linux doesn't/can't know about.

> Maxime

Thanks,
Samuel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 03/10] dt-bindings: mailbox: Add a sunxi message box binding
  2019-08-20  7:14   ` Maxime Ripard
@ 2019-08-20 13:04     ` Samuel Holland
  2019-08-21 12:07       ` Maxime Ripard
  0 siblings, 1 reply; 29+ messages in thread
From: Samuel Holland @ 2019-08-20 13:04 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, Corentin Labbe, Vasily Khoruzhick,
	devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Rob Herring

On 8/20/19 2:14 AM, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Aug 19, 2019 at 10:23:04PM -0500, Samuel Holland wrote:
>> This mailbox hardware is present in Allwinner sun8i, sun9i, and sun50i
>> SoCs. Add a device tree binding for it.
>>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>  .../mailbox/allwinner,sunxi-msgbox.yaml       | 79 +++++++++++++++++++
>>  1 file changed, 79 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
> 
> So we merged a bunch of schemas already, with the convention that the
> name was the first compatible to use that binding.
> 
> That would be allwinner,sun6i-a31-msgbox.yaml in that case

Okay, I'll rename the binding and driver (and Kconfig symbol?).

Thanks,
Samuel


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver
  2019-08-20 11:18   ` Ondřej Jirman
@ 2019-08-20 13:07     ` Samuel Holland
  2019-08-20 13:34       ` Ondřej Jirman
  2019-08-21 12:30       ` Maxime Ripard
  0 siblings, 2 replies; 29+ messages in thread
From: Samuel Holland @ 2019-08-20 13:07 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick, devicetree, linux-kernel, linux-sunxi,
	linux-clk, linux-arm-kernel

On 8/20/19 6:18 AM, Ondřej Jirman wrote:
> Hi Samuel,
> 
> On Mon, Aug 19, 2019 at 10:23:05PM -0500, Samuel Holland wrote:
>> Allwinner sun8i, sun9i, and sun50i SoCs contain a hardware message box
>> used for communication between the ARM CPUs and the ARISC management
>> coprocessor. The hardware contains 8 unidirectional 4-message FIFOs.
>>
>> Add a driver for it, so it can be used for SCPI or other communication
>> protocols.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>  drivers/mailbox/Kconfig        |  10 +
>>  drivers/mailbox/Makefile       |   2 +
>>  drivers/mailbox/sunxi-msgbox.c | 323 +++++++++++++++++++++++++++++++++
>>  3 files changed, 335 insertions(+)
>>  create mode 100644 drivers/mailbox/sunxi-msgbox.c
>>
>> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
>> index ab4eb750bbdd..57d12936175e 100644
>> --- a/drivers/mailbox/Kconfig
>> +++ b/drivers/mailbox/Kconfig
>> @@ -227,4 +227,14 @@ config ZYNQMP_IPI_MBOX
>>  	  message to the IPI buffer and will access the IPI control
>>  	  registers to kick the other processor or enquire status.
>>  
>> +config SUNXI_MSGBOX
>> +	tristate "Allwinner sunxi Message Box"
>> +	depends on ARCH_SUNXI || COMPILE_TEST
>> +	default ARCH_SUNXI
>> +	help
>> +	  Mailbox implementation for the hardware message box present in
>> +	  Allwinner sun8i, sun9i, and sun50i SoCs. The hardware message box is
>> +	  used for communication between the application CPUs and the power
>> +	  management coprocessor.
>> +
>>  endif
>> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
>> index c22fad6f696b..bec2d50b0976 100644
>> --- a/drivers/mailbox/Makefile
>> +++ b/drivers/mailbox/Makefile
>> @@ -48,3 +48,5 @@ obj-$(CONFIG_STM32_IPCC) 	+= stm32-ipcc.o
>>  obj-$(CONFIG_MTK_CMDQ_MBOX)	+= mtk-cmdq-mailbox.o
>>  
>>  obj-$(CONFIG_ZYNQMP_IPI_MBOX)	+= zynqmp-ipi-mailbox.o
>> +
>> +obj-$(CONFIG_SUNXI_MSGBOX)	+= sunxi-msgbox.o
>> diff --git a/drivers/mailbox/sunxi-msgbox.c b/drivers/mailbox/sunxi-msgbox.c
>> new file mode 100644
>> index 000000000000..29a5101a5390
>> --- /dev/null
>> +++ b/drivers/mailbox/sunxi-msgbox.c
>> @@ -0,0 +1,323 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +//
>> +// Copyright (c) 2017-2019 Samuel Holland <samuel@sholland.org>
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mailbox_controller.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset.h>
>> +#include <linux/spinlock.h>
>> +
>> +#define NUM_CHANS		8
>> +
>> +#define CTRL_REG(n)		(0x0000 + 0x4 * ((n) / 4))
>> +#define CTRL_RX(n)		BIT(0 + 8 * ((n) % 4))
>> +#define CTRL_TX(n)		BIT(4 + 8 * ((n) % 4))
>> +
>> +#define REMOTE_IRQ_EN_REG	0x0040
>> +#define REMOTE_IRQ_STAT_REG	0x0050
>> +#define LOCAL_IRQ_EN_REG	0x0060
>> +#define LOCAL_IRQ_STAT_REG	0x0070
>> +
>> +#define RX_IRQ(n)		BIT(0 + 2 * (n))
>> +#define RX_IRQ_MASK		0x5555
>> +#define TX_IRQ(n)		BIT(1 + 2 * (n))
>> +#define TX_IRQ_MASK		0xaaaa
>> +
>> +#define FIFO_STAT_REG(n)	(0x0100 + 0x4 * (n))
>> +#define FIFO_STAT_MASK		GENMASK(0, 0)
>> +
>> +#define MSG_STAT_REG(n)		(0x0140 + 0x4 * (n))
>> +#define MSG_STAT_MASK		GENMASK(2, 0)
>> +
>> +#define MSG_DATA_REG(n)		(0x0180 + 0x4 * (n))
>> +
>> +#define mbox_dbg(mbox, ...)	dev_dbg((mbox)->controller.dev, __VA_ARGS__)
>> +
>> +struct sunxi_msgbox {
>> +	struct mbox_controller controller;
>> +	struct clk *clk;
>> +	spinlock_t lock;
>> +	void __iomem *regs;
>> +};
>> +
>> +static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan);
>> +static bool sunxi_msgbox_peek_data(struct mbox_chan *chan);
>> +
>> +static inline int channel_number(struct mbox_chan *chan)
>> +{
>> +	return chan - chan->mbox->chans;
>> +}
>> +
>> +static inline struct sunxi_msgbox *channel_to_msgbox(struct mbox_chan *chan)
>> +{
>> +	return chan->con_priv;
>> +}
>> +
>> +static irqreturn_t sunxi_msgbox_irq(int irq, void *dev_id)
>> +{
>> +	struct sunxi_msgbox *mbox = dev_id;
>> +	uint32_t status;
>> +	int n;
>> +
>> +	/* Only examine channels that are currently enabled. */
>> +	status = readl(mbox->regs + LOCAL_IRQ_EN_REG) &
>> +		 readl(mbox->regs + LOCAL_IRQ_STAT_REG);
>> +
>> +	if (!(status & RX_IRQ_MASK))
>> +		return IRQ_NONE;
>> +
>> +	for (n = 0; n < NUM_CHANS; ++n) {
>> +		struct mbox_chan *chan = &mbox->controller.chans[n];
>> +
>> +		if (!(status & RX_IRQ(n)))
>> +			continue;
>> +
>> +		while (sunxi_msgbox_peek_data(chan)) {
>> +			uint32_t msg = readl(mbox->regs + MSG_DATA_REG(n));
>> +
>> +			mbox_dbg(mbox, "Channel %d received 0x%08x\n", n, msg);
>> +			mbox_chan_received_data(chan, &msg);
>> +		}
>> +
>> +		/* The IRQ can be cleared only once the FIFO is empty. */
>> +		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int sunxi_msgbox_send_data(struct mbox_chan *chan, void *data)
>> +{
>> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
>> +	int n = channel_number(chan);
>> +	uint32_t msg = *(uint32_t *)data;
>> +
>> +	/* Using a channel backwards gets the hardware into a bad state. */
>> +	if (WARN_ON_ONCE(!(readl(mbox->regs + CTRL_REG(n)) & CTRL_TX(n))))
>> +		return 0;
>> +
>> +	/* We cannot post a new message if the FIFO is full. */
>> +	if (readl(mbox->regs + FIFO_STAT_REG(n)) & FIFO_STAT_MASK) {
>> +		mbox_dbg(mbox, "Channel %d busy sending 0x%08x\n", n, msg);
>> +		return -EBUSY;
>> +	}
>> +
>> +	writel(msg, mbox->regs + MSG_DATA_REG(n));
>> +	mbox_dbg(mbox, "Channel %d sent 0x%08x\n", n, msg);
>> +
>> +	return 0;
>> +}
>> +
>> +static int sunxi_msgbox_startup(struct mbox_chan *chan)
>> +{
>> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
>> +	int n = channel_number(chan);
>> +
>> +	/* The coprocessor is responsible for setting channel directions. */
>> +	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
>> +		/* Flush the receive FIFO. */
>> +		while (sunxi_msgbox_peek_data(chan))
>> +			readl(mbox->regs + MSG_DATA_REG(n));
>> +		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
>> +
>> +		/* Enable the receive IRQ. */
>> +		spin_lock(&mbox->lock);
>> +		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) | RX_IRQ(n),
>> +		       mbox->regs + LOCAL_IRQ_EN_REG);
>> +		spin_unlock(&mbox->lock);
>> +	}
>> +
>> +	mbox_dbg(mbox, "Channel %d startup complete\n", n);
>> +
>> +	return 0;
>> +}
>> +
>> +static void sunxi_msgbox_shutdown(struct mbox_chan *chan)
>> +{
>> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
>> +	int n = channel_number(chan);
>> +
>> +	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
>> +		/* Disable the receive IRQ. */
>> +		spin_lock(&mbox->lock);
>> +		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) & ~RX_IRQ(n),
>> +		       mbox->regs + LOCAL_IRQ_EN_REG);
>> +		spin_unlock(&mbox->lock);
>> +
>> +		/* Attempt to flush the FIFO until the IRQ is cleared. */
>> +		do {
>> +			while (sunxi_msgbox_peek_data(chan))
>> +				readl(mbox->regs + MSG_DATA_REG(n));
>> +			writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
>> +		} while (readl(mbox->regs + LOCAL_IRQ_STAT_REG) & RX_IRQ(n));
>> +	}
>> +
>> +	mbox_dbg(mbox, "Channel %d shutdown complete\n", n);
>> +}
>> +
>> +static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan)
>> +{
>> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
>> +	int n = channel_number(chan);
>> +
>> +	/*
>> +	 * The hardware allows snooping on the remote user's IRQ statuses.
>> +	 * We consider a message to be acknowledged only once the receive IRQ
>> +	 * for that channel is cleared. Since the receive IRQ for a channel
>> +	 * cannot be cleared until the FIFO for that channel is empty, this
>> +	 * ensures that the message has actually been read. It also gives the
>> +	 * recipient an opportunity to perform minimal processing before
>> +	 * acknowledging the message.
>> +	 */
>> +	return !(readl(mbox->regs + REMOTE_IRQ_STAT_REG) & RX_IRQ(n));
>> +}
>> +
>> +static bool sunxi_msgbox_peek_data(struct mbox_chan *chan)
>> +{
>> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
>> +	int n = channel_number(chan);
>> +
>> +	return readl(mbox->regs + MSG_STAT_REG(n)) & MSG_STAT_MASK;
>> +}
>> +
>> +static const struct mbox_chan_ops sunxi_msgbox_chan_ops = {
>> +	.send_data    = sunxi_msgbox_send_data,
>> +	.startup      = sunxi_msgbox_startup,
>> +	.shutdown     = sunxi_msgbox_shutdown,
>> +	.last_tx_done = sunxi_msgbox_last_tx_done,
>> +	.peek_data    = sunxi_msgbox_peek_data,
>> +};
>> +
>> +static int sunxi_msgbox_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct mbox_chan *chans;
>> +	struct reset_control *reset;
>> +	struct resource *res;
>> +	struct sunxi_msgbox *mbox;
>> +	int i, ret;
>> +
>> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
>> +	if (!mbox)
>> +		return -ENOMEM;
>> +
>> +	chans = devm_kcalloc(dev, NUM_CHANS, sizeof(*chans), GFP_KERNEL);
>> +	if (!chans)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < NUM_CHANS; ++i)
>> +		chans[i].con_priv = mbox;
>> +
>> +	mbox->clk = devm_clk_get(dev, NULL);
>> +	if (IS_ERR(mbox->clk)) {
>> +		ret = PTR_ERR(mbox->clk);
>> +		dev_err(dev, "Failed to get clock: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_prepare_enable(mbox->clk);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to enable clock: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	reset = devm_reset_control_get(dev, NULL);
>> +	if (IS_ERR(reset)) {
>> +		ret = PTR_ERR(reset);
>> +		dev_err(dev, "Failed to get reset control: %d\n", ret);
>> +		goto err_disable_unprepare;
>> +	}
>> +
>> +	ret = reset_control_deassert(reset);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to deassert reset: %d\n", ret);
>> +		goto err_disable_unprepare;
>> +	}
> 
> You need to assert the reset again from now on, in error paths. devm
> will not do that for you.

I know, and that's intentional. This same message box device is used for ATF to
communicate with SCP firmware (on a different channel). This could be happening
on a different core while Linux is running. So Linux is not allowed to deassert
the reset. clk_disable_unprepare() is only okay because the clock is marked as
critical.

>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res) {
>> +		ret = -ENODEV;
>> +		goto err_disable_unprepare;
>> +	}
>> +
>> +	mbox->regs = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(mbox->regs)) {
>> +		ret = PTR_ERR(mbox->regs);
>> +		dev_err(dev, "Failed to map MMIO resource: %d\n", ret);
>> +		goto err_disable_unprepare;
>> +	}
>> +
>> +	/* Disable all IRQs for this end of the msgbox. */
>> +	writel(0, mbox->regs + LOCAL_IRQ_EN_REG);
>> +
>> +	ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
>> +			       sunxi_msgbox_irq, 0, dev_name(dev), mbox);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to register IRQ handler: %d\n", ret);
>> +		goto err_disable_unprepare;
>> +	}
>> +
>> +	mbox->controller.dev           = dev;
>> +	mbox->controller.ops           = &sunxi_msgbox_chan_ops;
>> +	mbox->controller.chans         = chans;
>> +	mbox->controller.num_chans     = NUM_CHANS;
>> +	mbox->controller.txdone_irq    = false;
>> +	mbox->controller.txdone_poll   = true;
>> +	mbox->controller.txpoll_period = 5;
>> +
>> +	spin_lock_init(&mbox->lock);
>> +	platform_set_drvdata(pdev, mbox);
>> +
>> +	ret = mbox_controller_register(&mbox->controller);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to register controller: %d\n", ret);
>> +		goto err_disable_unprepare;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_disable_unprepare:
>> +	clk_disable_unprepare(mbox->clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static int sunxi_msgbox_remove(struct platform_device *pdev)
>> +{
>> +	struct sunxi_msgbox *mbox = platform_get_drvdata(pdev);
>> +
>> +	mbox_controller_unregister(&mbox->controller);
>> +	clk_disable_unprepare(mbox->clk);
> 
> Also, assert the reset here.

Same comment as above. This is intentional.

Thanks,
Samuel

>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id sunxi_msgbox_of_match[] = {
>> +	{ .compatible = "allwinner,sun6i-a31-msgbox", },
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, sunxi_msgbox_of_match);
>> +
>> +static struct platform_driver sunxi_msgbox_driver = {
>> +	.driver = {
>> +		.name = "sunxi-msgbox",
>> +		.of_match_table = sunxi_msgbox_of_match,
>> +	},
>> +	.probe  = sunxi_msgbox_probe,
>> +	.remove = sunxi_msgbox_remove,
>> +};
>> +module_platform_driver(sunxi_msgbox_driver);
>> +
>> +MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
>> +MODULE_DESCRIPTION("Allwinner sunxi Message Box");
>> +MODULE_LICENSE("GPL v2");
>> -- 
>> 2.21.0

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 05/10] ARM: dts: sunxi: a80: Add msgbox node
  2019-08-20  8:15   ` Maxime Ripard
@ 2019-08-20 13:17     ` Samuel Holland
  2019-08-23 14:56       ` Maxime Ripard
  0 siblings, 1 reply; 29+ messages in thread
From: Samuel Holland @ 2019-08-20 13:17 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, Corentin Labbe, Vasily Khoruzhick,
	devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi

Hi,

On 8/20/19 3:15 AM, Maxime Ripard wrote:
> On Mon, Aug 19, 2019 at 10:23:06PM -0500, Samuel Holland wrote:
>> The A80 SoC contains a message box that can be used to send messages and
>> interrupts back and forth between the ARM application CPUs and the ARISC
>> coprocessor. Add a device tree node for it.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
> 
> I think you mentionned that crust has been tested only on the A64 and
> the H3/H5, did you test the mailbox on those other SoCs as well?

No, I only have A64/H3/H5, and recently H6, hardware to test. I've looked
through the manuals to verify that the registers are all the same, but I haven't
run the driver on earlier SoCs.

On 32-bit SoCs, where there's no other user of SRAM A2, it should be easy to get
the toy firmware running. All you should need to do is:
  1) Update the MMIO base/clock addresses in drivers/msgbox/sunxi-msgbox.c
  2) Update the load address in platform/sun50i/include/platform/memory.h
  3) Load the firmware to SRAM A2 (can be done from a U-Boot shell)
  4) Initialize the reset vector (algorithm is in tools/test.c:109)
  5) Deassert AR100 reset (again, these last two steps can be done from U-Boot)

Thanks,
Samuel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver
  2019-08-20 13:07     ` Samuel Holland
@ 2019-08-20 13:34       ` Ondřej Jirman
  2019-08-21 12:30       ` Maxime Ripard
  1 sibling, 0 replies; 29+ messages in thread
From: Ondřej Jirman @ 2019-08-20 13:34 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick, devicetree, linux-kernel, linux-sunxi,
	linux-clk, linux-arm-kernel

Hi,

On Tue, Aug 20, 2019 at 08:07:53AM -0500, Samuel Holland wrote:
> On 8/20/19 6:18 AM, Ondřej Jirman wrote:
> > Hi Samuel,
> > 
> > On Mon, Aug 19, 2019 at 10:23:05PM -0500, Samuel Holland wrote:
> >> Allwinner sun8i, sun9i, and sun50i SoCs contain a hardware message box
> >> used for communication between the ARM CPUs and the ARISC management
> >> coprocessor. The hardware contains 8 unidirectional 4-message FIFOs.
> >>
> >> Add a driver for it, so it can be used for SCPI or other communication
> >> protocols.
> >>
> >> Signed-off-by: Samuel Holland <samuel@sholland.org>
> >> ---
> >>  drivers/mailbox/Kconfig        |  10 +
> >>  drivers/mailbox/Makefile       |   2 +
> >>  drivers/mailbox/sunxi-msgbox.c | 323 +++++++++++++++++++++++++++++++++
> >>  3 files changed, 335 insertions(+)
> >>  create mode 100644 drivers/mailbox/sunxi-msgbox.c
> >>
> >> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> >> index ab4eb750bbdd..57d12936175e 100644
> >> --- a/drivers/mailbox/Kconfig
> >> +++ b/drivers/mailbox/Kconfig
> >> @@ -227,4 +227,14 @@ config ZYNQMP_IPI_MBOX
> >>  	  message to the IPI buffer and will access the IPI control
> >>  	  registers to kick the other processor or enquire status.
> >>  
> >> +config SUNXI_MSGBOX
> >> +	tristate "Allwinner sunxi Message Box"
> >> +	depends on ARCH_SUNXI || COMPILE_TEST
> >> +	default ARCH_SUNXI
> >> +	help
> >> +	  Mailbox implementation for the hardware message box present in
> >> +	  Allwinner sun8i, sun9i, and sun50i SoCs. The hardware message box is
> >> +	  used for communication between the application CPUs and the power
> >> +	  management coprocessor.
> >> +
> >>  endif
> >> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> >> index c22fad6f696b..bec2d50b0976 100644
> >> --- a/drivers/mailbox/Makefile
> >> +++ b/drivers/mailbox/Makefile
> >> @@ -48,3 +48,5 @@ obj-$(CONFIG_STM32_IPCC) 	+= stm32-ipcc.o
> >>  obj-$(CONFIG_MTK_CMDQ_MBOX)	+= mtk-cmdq-mailbox.o
> >>  
> >>  obj-$(CONFIG_ZYNQMP_IPI_MBOX)	+= zynqmp-ipi-mailbox.o
> >> +
> >> +obj-$(CONFIG_SUNXI_MSGBOX)	+= sunxi-msgbox.o
> >> diff --git a/drivers/mailbox/sunxi-msgbox.c b/drivers/mailbox/sunxi-msgbox.c
> >> new file mode 100644
> >> index 000000000000..29a5101a5390
> >> --- /dev/null
> >> +++ b/drivers/mailbox/sunxi-msgbox.c
> >> @@ -0,0 +1,323 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +//
> >> +// Copyright (c) 2017-2019 Samuel Holland <samuel@sholland.org>
> >> +
> >> +#include <linux/bitops.h>
> >> +#include <linux/clk.h>
> >> +#include <linux/device.h>
> >> +#include <linux/err.h>
> >> +#include <linux/interrupt.h>
> >> +#include <linux/io.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/mailbox_controller.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of.h>
> >> +#include <linux/of_irq.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/reset.h>
> >> +#include <linux/spinlock.h>
> >> +
> >> +#define NUM_CHANS		8
> >> +
> >> +#define CTRL_REG(n)		(0x0000 + 0x4 * ((n) / 4))
> >> +#define CTRL_RX(n)		BIT(0 + 8 * ((n) % 4))
> >> +#define CTRL_TX(n)		BIT(4 + 8 * ((n) % 4))
> >> +
> >> +#define REMOTE_IRQ_EN_REG	0x0040
> >> +#define REMOTE_IRQ_STAT_REG	0x0050
> >> +#define LOCAL_IRQ_EN_REG	0x0060
> >> +#define LOCAL_IRQ_STAT_REG	0x0070
> >> +
> >> +#define RX_IRQ(n)		BIT(0 + 2 * (n))
> >> +#define RX_IRQ_MASK		0x5555
> >> +#define TX_IRQ(n)		BIT(1 + 2 * (n))
> >> +#define TX_IRQ_MASK		0xaaaa
> >> +
> >> +#define FIFO_STAT_REG(n)	(0x0100 + 0x4 * (n))
> >> +#define FIFO_STAT_MASK		GENMASK(0, 0)
> >> +
> >> +#define MSG_STAT_REG(n)		(0x0140 + 0x4 * (n))
> >> +#define MSG_STAT_MASK		GENMASK(2, 0)
> >> +
> >> +#define MSG_DATA_REG(n)		(0x0180 + 0x4 * (n))
> >> +
> >> +#define mbox_dbg(mbox, ...)	dev_dbg((mbox)->controller.dev, __VA_ARGS__)
> >> +
> >> +struct sunxi_msgbox {
> >> +	struct mbox_controller controller;
> >> +	struct clk *clk;
> >> +	spinlock_t lock;
> >> +	void __iomem *regs;
> >> +};
> >> +
> >> +static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan);
> >> +static bool sunxi_msgbox_peek_data(struct mbox_chan *chan);
> >> +
> >> +static inline int channel_number(struct mbox_chan *chan)
> >> +{
> >> +	return chan - chan->mbox->chans;
> >> +}
> >> +
> >> +static inline struct sunxi_msgbox *channel_to_msgbox(struct mbox_chan *chan)
> >> +{
> >> +	return chan->con_priv;
> >> +}
> >> +
> >> +static irqreturn_t sunxi_msgbox_irq(int irq, void *dev_id)
> >> +{
> >> +	struct sunxi_msgbox *mbox = dev_id;
> >> +	uint32_t status;
> >> +	int n;
> >> +
> >> +	/* Only examine channels that are currently enabled. */
> >> +	status = readl(mbox->regs + LOCAL_IRQ_EN_REG) &
> >> +		 readl(mbox->regs + LOCAL_IRQ_STAT_REG);
> >> +
> >> +	if (!(status & RX_IRQ_MASK))
> >> +		return IRQ_NONE;
> >> +
> >> +	for (n = 0; n < NUM_CHANS; ++n) {
> >> +		struct mbox_chan *chan = &mbox->controller.chans[n];
> >> +
> >> +		if (!(status & RX_IRQ(n)))
> >> +			continue;
> >> +
> >> +		while (sunxi_msgbox_peek_data(chan)) {
> >> +			uint32_t msg = readl(mbox->regs + MSG_DATA_REG(n));
> >> +
> >> +			mbox_dbg(mbox, "Channel %d received 0x%08x\n", n, msg);
> >> +			mbox_chan_received_data(chan, &msg);
> >> +		}
> >> +
> >> +		/* The IRQ can be cleared only once the FIFO is empty. */
> >> +		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
> >> +	}
> >> +
> >> +	return IRQ_HANDLED;
> >> +}
> >> +
> >> +static int sunxi_msgbox_send_data(struct mbox_chan *chan, void *data)
> >> +{
> >> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> >> +	int n = channel_number(chan);
> >> +	uint32_t msg = *(uint32_t *)data;
> >> +
> >> +	/* Using a channel backwards gets the hardware into a bad state. */
> >> +	if (WARN_ON_ONCE(!(readl(mbox->regs + CTRL_REG(n)) & CTRL_TX(n))))
> >> +		return 0;
> >> +
> >> +	/* We cannot post a new message if the FIFO is full. */
> >> +	if (readl(mbox->regs + FIFO_STAT_REG(n)) & FIFO_STAT_MASK) {
> >> +		mbox_dbg(mbox, "Channel %d busy sending 0x%08x\n", n, msg);
> >> +		return -EBUSY;
> >> +	}
> >> +
> >> +	writel(msg, mbox->regs + MSG_DATA_REG(n));
> >> +	mbox_dbg(mbox, "Channel %d sent 0x%08x\n", n, msg);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int sunxi_msgbox_startup(struct mbox_chan *chan)
> >> +{
> >> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> >> +	int n = channel_number(chan);
> >> +
> >> +	/* The coprocessor is responsible for setting channel directions. */
> >> +	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
> >> +		/* Flush the receive FIFO. */
> >> +		while (sunxi_msgbox_peek_data(chan))
> >> +			readl(mbox->regs + MSG_DATA_REG(n));
> >> +		writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
> >> +
> >> +		/* Enable the receive IRQ. */
> >> +		spin_lock(&mbox->lock);
> >> +		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) | RX_IRQ(n),
> >> +		       mbox->regs + LOCAL_IRQ_EN_REG);
> >> +		spin_unlock(&mbox->lock);
> >> +	}
> >> +
> >> +	mbox_dbg(mbox, "Channel %d startup complete\n", n);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void sunxi_msgbox_shutdown(struct mbox_chan *chan)
> >> +{
> >> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> >> +	int n = channel_number(chan);
> >> +
> >> +	if (readl(mbox->regs + CTRL_REG(n)) & CTRL_RX(n)) {
> >> +		/* Disable the receive IRQ. */
> >> +		spin_lock(&mbox->lock);
> >> +		writel(readl(mbox->regs + LOCAL_IRQ_EN_REG) & ~RX_IRQ(n),
> >> +		       mbox->regs + LOCAL_IRQ_EN_REG);
> >> +		spin_unlock(&mbox->lock);
> >> +
> >> +		/* Attempt to flush the FIFO until the IRQ is cleared. */
> >> +		do {
> >> +			while (sunxi_msgbox_peek_data(chan))
> >> +				readl(mbox->regs + MSG_DATA_REG(n));
> >> +			writel(RX_IRQ(n), mbox->regs + LOCAL_IRQ_STAT_REG);
> >> +		} while (readl(mbox->regs + LOCAL_IRQ_STAT_REG) & RX_IRQ(n));
> >> +	}
> >> +
> >> +	mbox_dbg(mbox, "Channel %d shutdown complete\n", n);
> >> +}
> >> +
> >> +static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan)
> >> +{
> >> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> >> +	int n = channel_number(chan);
> >> +
> >> +	/*
> >> +	 * The hardware allows snooping on the remote user's IRQ statuses.
> >> +	 * We consider a message to be acknowledged only once the receive IRQ
> >> +	 * for that channel is cleared. Since the receive IRQ for a channel
> >> +	 * cannot be cleared until the FIFO for that channel is empty, this
> >> +	 * ensures that the message has actually been read. It also gives the
> >> +	 * recipient an opportunity to perform minimal processing before
> >> +	 * acknowledging the message.
> >> +	 */
> >> +	return !(readl(mbox->regs + REMOTE_IRQ_STAT_REG) & RX_IRQ(n));
> >> +}
> >> +
> >> +static bool sunxi_msgbox_peek_data(struct mbox_chan *chan)
> >> +{
> >> +	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
> >> +	int n = channel_number(chan);
> >> +
> >> +	return readl(mbox->regs + MSG_STAT_REG(n)) & MSG_STAT_MASK;
> >> +}
> >> +
> >> +static const struct mbox_chan_ops sunxi_msgbox_chan_ops = {
> >> +	.send_data    = sunxi_msgbox_send_data,
> >> +	.startup      = sunxi_msgbox_startup,
> >> +	.shutdown     = sunxi_msgbox_shutdown,
> >> +	.last_tx_done = sunxi_msgbox_last_tx_done,
> >> +	.peek_data    = sunxi_msgbox_peek_data,
> >> +};
> >> +
> >> +static int sunxi_msgbox_probe(struct platform_device *pdev)
> >> +{
> >> +	struct device *dev = &pdev->dev;
> >> +	struct mbox_chan *chans;
> >> +	struct reset_control *reset;
> >> +	struct resource *res;
> >> +	struct sunxi_msgbox *mbox;
> >> +	int i, ret;
> >> +
> >> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> >> +	if (!mbox)
> >> +		return -ENOMEM;
> >> +
> >> +	chans = devm_kcalloc(dev, NUM_CHANS, sizeof(*chans), GFP_KERNEL);
> >> +	if (!chans)
> >> +		return -ENOMEM;
> >> +
> >> +	for (i = 0; i < NUM_CHANS; ++i)
> >> +		chans[i].con_priv = mbox;
> >> +
> >> +	mbox->clk = devm_clk_get(dev, NULL);
> >> +	if (IS_ERR(mbox->clk)) {
> >> +		ret = PTR_ERR(mbox->clk);
> >> +		dev_err(dev, "Failed to get clock: %d\n", ret);
> >> +		return ret;
> >> +	}
> >> +
> >> +	ret = clk_prepare_enable(mbox->clk);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to enable clock: %d\n", ret);
> >> +		return ret;
> >> +	}
> >> +
> >> +	reset = devm_reset_control_get(dev, NULL);
> >> +	if (IS_ERR(reset)) {
> >> +		ret = PTR_ERR(reset);
> >> +		dev_err(dev, "Failed to get reset control: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	ret = reset_control_deassert(reset);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to deassert reset: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> > 
> > You need to assert the reset again from now on, in error paths. devm
> > will not do that for you.
> 
> I know, and that's intentional. This same message box device is used for ATF to
> communicate with SCP firmware (on a different channel). This could be happening
> on a different core while Linux is running. So Linux is not allowed to deassert
> the reset. clk_disable_unprepare() is only okay because the clock is marked as
> critical.

Okay. It needs to be docummented here though, so that someone will
not "fix" it in the future, after finding this with coccinelle or
something.

regards,
	o.

> >> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> +	if (!res) {
> >> +		ret = -ENODEV;
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	mbox->regs = devm_ioremap_resource(&pdev->dev, res);
> >> +	if (IS_ERR(mbox->regs)) {
> >> +		ret = PTR_ERR(mbox->regs);
> >> +		dev_err(dev, "Failed to map MMIO resource: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	/* Disable all IRQs for this end of the msgbox. */
> >> +	writel(0, mbox->regs + LOCAL_IRQ_EN_REG);
> >> +
> >> +	ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
> >> +			       sunxi_msgbox_irq, 0, dev_name(dev), mbox);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to register IRQ handler: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	mbox->controller.dev           = dev;
> >> +	mbox->controller.ops           = &sunxi_msgbox_chan_ops;
> >> +	mbox->controller.chans         = chans;
> >> +	mbox->controller.num_chans     = NUM_CHANS;
> >> +	mbox->controller.txdone_irq    = false;
> >> +	mbox->controller.txdone_poll   = true;
> >> +	mbox->controller.txpoll_period = 5;
> >> +
> >> +	spin_lock_init(&mbox->lock);
> >> +	platform_set_drvdata(pdev, mbox);
> >> +
> >> +	ret = mbox_controller_register(&mbox->controller);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to register controller: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	return 0;
> >> +
> >> +err_disable_unprepare:
> >> +	clk_disable_unprepare(mbox->clk);
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int sunxi_msgbox_remove(struct platform_device *pdev)
> >> +{
> >> +	struct sunxi_msgbox *mbox = platform_get_drvdata(pdev);
> >> +
> >> +	mbox_controller_unregister(&mbox->controller);
> >> +	clk_disable_unprepare(mbox->clk);
> > 
> > Also, assert the reset here.
> 
> Same comment as above. This is intentional.
> 
> Thanks,
> Samuel
> 
> >> +	return 0;
> >> +}
> >> +
> >> +static const struct of_device_id sunxi_msgbox_of_match[] = {
> >> +	{ .compatible = "allwinner,sun6i-a31-msgbox", },
> >> +	{},
> >> +};
> >> +MODULE_DEVICE_TABLE(of, sunxi_msgbox_of_match);
> >> +
> >> +static struct platform_driver sunxi_msgbox_driver = {
> >> +	.driver = {
> >> +		.name = "sunxi-msgbox",
> >> +		.of_match_table = sunxi_msgbox_of_match,
> >> +	},
> >> +	.probe  = sunxi_msgbox_probe,
> >> +	.remove = sunxi_msgbox_remove,
> >> +};
> >> +module_platform_driver(sunxi_msgbox_driver);
> >> +
> >> +MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
> >> +MODULE_DESCRIPTION("Allwinner sunxi Message Box");
> >> +MODULE_LICENSE("GPL v2");
> >> -- 
> >> 2.21.0
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 03/10] dt-bindings: mailbox: Add a sunxi message box binding
  2019-08-20 13:04     ` Samuel Holland
@ 2019-08-21 12:07       ` Maxime Ripard
  0 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2019-08-21 12:07 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, Corentin Labbe, Vasily Khoruzhick,
	devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi, Rob Herring

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On Tue, Aug 20, 2019 at 08:04:26AM -0500, Samuel Holland wrote:
> On 8/20/19 2:14 AM, Maxime Ripard wrote:
> > Hi,
> >
> > On Mon, Aug 19, 2019 at 10:23:04PM -0500, Samuel Holland wrote:
> >> This mailbox hardware is present in Allwinner sun8i, sun9i, and sun50i
> >> SoCs. Add a device tree binding for it.
> >>
> >> Reviewed-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Samuel Holland <samuel@sholland.org>
> >> ---
> >>  .../mailbox/allwinner,sunxi-msgbox.yaml       | 79 +++++++++++++++++++
> >>  1 file changed, 79 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
> >
> > So we merged a bunch of schemas already, with the convention that the
> > name was the first compatible to use that binding.
> >
> > That would be allwinner,sun6i-a31-msgbox.yaml in that case
>
> Okay, I'll rename the binding and driver (and Kconfig symbol?).

Yep, thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 02/10] clk: sunxi-ng: Mark AR100 clocks as critical
  2019-08-20 13:02     ` Samuel Holland
@ 2019-08-21 12:24       ` Maxime Ripard
  2019-09-05 18:56         ` Stephen Boyd
  0 siblings, 1 reply; 29+ messages in thread
From: Maxime Ripard @ 2019-08-21 12:24 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, Corentin Labbe, Vasily Khoruzhick,
	devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi

[-- Attachment #1: Type: text/plain, Size: 4574 bytes --]

On Tue, Aug 20, 2019 at 08:02:55AM -0500, Samuel Holland wrote:
> On 8/20/19 2:11 AM, Maxime Ripard wrote:
> > On Mon, Aug 19, 2019 at 10:23:03PM -0500, Samuel Holland wrote:
> >> On sun8i, sun9i, and sun50i SoCs, system suspend/resume support requires
> >> firmware running on the AR100 coprocessor (the "SCP"). Such firmware can
> >> provide additional features, such as thermal monitoring and poweron/off
> >> support for boards without a PMIC.
> >>
> >> Since the AR100 may be running critical firmware, even if Linux does not
> >> know about it or directly interact with it (all requests may go through
> >> an intermediary interface such as PSCI), Linux must not turn off its
> >> clock.
>
> This paragraph here is the key. The firmware won't necessarily have a device
> tree node, and in the current design it will not, since Linux never communicates
> with it directly. All communication goes through ATF via PSCI.

Sorry, I'm a bit lost in all those ARM firmware interfaces.

I thought SCPI was supposed to be a separate interface that had
nothing to do with PSCI?

Anyway, my main concern is that I don't really want to make exceptions
to the clock handling for everyone's usecase, and this creates a
precedent (and not even a permanent one, since eventually the plan is
to have all the clock handling happening in the firmware, right?).

There's the protected-clocks property in the DT though that will
achieve the same goal. The code to deal with it is not generic at the
moment, but it could be made that way. Would patching the DT to
protect the clock you care about, when you care about protecting them,
be an option for you?

> >> At this time, such power management firmware only exists for the A64 and
> >> H5 SoCs.  However, it makes sense to take care of all CCU drivers now
> >> for consistency, and to ease the transition in the future once firmware
> >> is ported to the other SoCs.
> >>
> >> Leaving the clock running is safe even if no firmware is present, since
> >> the AR100 stays in reset by default. In most cases, the AR100 clock is
> >> kept enabled by Linux anyway, since it is the parent of all APB0 bus
> >> peripherals. This change only prevents Linux from turning off the AR100
> >> clock in the rare case that no peripherals are in use.
> >>
> >> Signed-off-by: Samuel Holland <samuel@sholland.org>
> >
> > So I'm not really sure where you want to go with this.
> >
> > That clock is only useful where you're having a firmware running on
> > the AR100, and that firmware would have a device tree node of its own,
> > where we could list the clocks needed for the firmware to keep
> > running, if it ever runs. If the driver has not been compiled in /
> > loaded, then we don't care either.
>
> See above. I don't expect that the firmware would have a device tree node,
> because the firmware doesn't need any Linux drivers.
>
> > But more fundamentally, if we're going to use SCPI, then those clocks
> > will not be handled by that driver anyway, but by the firmware, right?
>
> In the future, we might use SCPI clocks/sensors/regulators/etc. from Linux, but
> that's not the plan at the moment. Given that it's already been two years since
> I started this project, I'm trying to limit its scope so I can get at least some
> part merged. The first step is to integrate a firmware that provides
> suspend/resume functionality only. That firmware does implement SCPI, and if the
> top-level Linux SCPI driver worked with multiple mailbox channels, it could
> query the firmware's version and fetures. But all of the SCPI commands used for
> suspend/resume must go through ATF via PSCI.

I didn't know that you could / should do that with PSCI / SCPI.

> > So I'm not really sure that we should do it statically this way, and
> > that we should do it at all.
>
> Do you have a better way to model "firmware uses this clock behind the scenes,
> so Linux please don't touch it"? It's unfortunate that we have Linux and
> firmware fighting over the R_CCU, but since we didn't have firmware (e.g. SCPI
> clocks) in the beginning, it's where we are today.
>
> The AR100 clock doesn't actually have a gate, and it generally has dependencies
> like R_INTC in use. So as I mentioned in the commit message, the clock will
> normally be on anyway. The goal was to model the fact that there are users of
> this clock that Linux doesn't/can't know about.

Like I said, if that's an option, I'd prefer to have protected-clocks
work for everyone / for sunxi.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver
  2019-08-20 13:07     ` Samuel Holland
  2019-08-20 13:34       ` Ondřej Jirman
@ 2019-08-21 12:30       ` Maxime Ripard
  1 sibling, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2019-08-21 12:30 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, Corentin Labbe, Vasily Khoruzhick,
	devicetree, linux-kernel, linux-sunxi, linux-clk,
	linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]

On Tue, Aug 20, 2019 at 08:07:53AM -0500, Samuel Holland wrote:
> On 8/20/19 6:18 AM, Ondřej Jirman wrote:
> >> +	reset = devm_reset_control_get(dev, NULL);
> >> +	if (IS_ERR(reset)) {
> >> +		ret = PTR_ERR(reset);
> >> +		dev_err(dev, "Failed to get reset control: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >> +
> >> +	ret = reset_control_deassert(reset);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to deassert reset: %d\n", ret);
> >> +		goto err_disable_unprepare;
> >> +	}
> >
> > You need to assert the reset again from now on, in error paths. devm
> > will not do that for you.
>
> I know, and that's intentional. This same message box device is used for ATF to
> communicate with SCP firmware (on a different channel). This could be happening
> on a different core while Linux is running. So Linux is not allowed to deassert
> the reset. clk_disable_unprepare() is only okay because the clock is marked as
> critical.

I agree with Ondrej that since this is clearly not the standard use of
the API, this must have a big comment explaining why we're doing it
this way.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 05/10] ARM: dts: sunxi: a80: Add msgbox node
  2019-08-20 13:17     ` Samuel Holland
@ 2019-08-23 14:56       ` Maxime Ripard
  0 siblings, 0 replies; 29+ messages in thread
From: Maxime Ripard @ 2019-08-23 14:56 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Stephen Boyd,
	Rob Herring, Mark Rutland, Corentin Labbe, Vasily Khoruzhick,
	devicetree, linux-arm-kernel, linux-clk, linux-kernel,
	linux-sunxi

[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]

Hi,

On Tue, Aug 20, 2019 at 08:17:49AM -0500, Samuel Holland wrote:
> On 8/20/19 3:15 AM, Maxime Ripard wrote:
> > On Mon, Aug 19, 2019 at 10:23:06PM -0500, Samuel Holland wrote:
> >> The A80 SoC contains a message box that can be used to send messages and
> >> interrupts back and forth between the ARM application CPUs and the ARISC
> >> coprocessor. Add a device tree node for it.
> >>
> >> Signed-off-by: Samuel Holland <samuel@sholland.org>
> >
> > I think you mentionned that crust has been tested only on the A64 and
> > the H3/H5, did you test the mailbox on those other SoCs as well?
>
> No, I only have A64/H3/H5, and recently H6, hardware to test. I've looked
> through the manuals to verify that the registers are all the same, but I haven't
> run the driver on earlier SoCs.

I'd rather not merge them until they've been properly tested. We've
had some surprises with the documentation in the past :/

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 02/10] clk: sunxi-ng: Mark AR100 clocks as critical
  2019-08-21 12:24       ` Maxime Ripard
@ 2019-09-05 18:56         ` Stephen Boyd
  0 siblings, 0 replies; 29+ messages in thread
From: Stephen Boyd @ 2019-09-05 18:56 UTC (permalink / raw)
  To: Maxime Ripard, Samuel Holland
  Cc: Chen-Yu Tsai, Jassi Brar, Michael Turquette, Rob Herring,
	Mark Rutland, Corentin Labbe, Vasily Khoruzhick, devicetree,
	linux-arm-kernel, linux-clk, linux-kernel, linux-sunxi

Quoting Maxime Ripard (2019-08-21 05:24:36)
> On Tue, Aug 20, 2019 at 08:02:55AM -0500, Samuel Holland wrote:
> > On 8/20/19 2:11 AM, Maxime Ripard wrote:
> > > So I'm not really sure that we should do it statically this way, and
> > > that we should do it at all.
> >
> > Do you have a better way to model "firmware uses this clock behind the scenes,
> > so Linux please don't touch it"? It's unfortunate that we have Linux and
> > firmware fighting over the R_CCU, but since we didn't have firmware (e.g. SCPI
> > clocks) in the beginning, it's where we are today.
> >
> > The AR100 clock doesn't actually have a gate, and it generally has dependencies
> > like R_INTC in use. So as I mentioned in the commit message, the clock will
> > normally be on anyway. The goal was to model the fact that there are users of
> > this clock that Linux doesn't/can't know about.
> 
> Like I said, if that's an option, I'd prefer to have protected-clocks
> work for everyone / for sunxi.
> 

Yes. Use protected-clocks to indicate what shouldn't be touched by the
kernel. It's not super easy to make it "generic" right now, but I
suppose we can work the flag into the core framework more so that we
still register the clks but otherwise make the 'clk_get()' operation
fail on them somehow and the disable unused operation skip them. I just
took the easy way out for qcom for the time being and didn't register
them from the driver.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 00/10] Allwinner sunxi message box support
  2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
                   ` (9 preceding siblings ...)
  2019-08-20  3:23 ` [PATCH v4 10/10] [DO NOT MERGE] drivers: firmware: msgbox demo Samuel Holland
@ 2019-09-09  3:22 ` Ondřej Jirman
  2019-09-09  3:54   ` Samuel Holland
  10 siblings, 1 reply; 29+ messages in thread
From: Ondřej Jirman @ 2019-09-09  3:22 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick, devicetree, linux-kernel, linux-sunxi,
	linux-clk, linux-arm-kernel

Hello Samuel,

On Mon, Aug 19, 2019 at 10:23:01PM -0500, Samuel Holland wrote:
> This series adds support for the "hardware message box" in sun8i, sun9i,
> and sun50i SoCs, used for communication with the ARISC management
> processor (the platform's equivalent of the ARM SCP). The end goal is to
> use the arm_scpi driver as a client, communicating with firmware running
> on the AR100 CPU, or to use the mailbox to forward NMIs that the
> firmware picks up from R_INTC.
> 
> Unfortunately, the ARM SCPI client no longer works with this driver
> since it now exposes all 8 hardware FIFOs individually. The SCPI client
> could be made to work (and I posted proof-of-concept code to that effect
> with v1 of this series), but that is a low priority, as Linux does not
> directly use SCPI with the current firmware version; all SCPI use goes
> through ATF via PSCI.
> 
> As requested in the comments to v3 of this patchset, a demo client is
> provided in the final patch. This demo goes along with a toy firmware
> which shows that the driver does indeed work for two-way communication
> on all channels. To build the firmware component, run:

I've tried using this driver with mainline arm_scpi driver (which is probably
an expected future use, since crust provides SCPI interface).

The problem I've found is that arm_scpi expects message box to be
bi-directional, but this driver provides uni-directional interface.

What do you think about making this driver provide bi-directional interface?
We could halve the number of channels to 4 and mandate TX/RX configuration
(from main CPU's PoV) as ABI.

Otherwise it's impossible to use it with the arm_scpi driver.

Or do you have any other ideas? I guess arm_scpi can be fixed to add a
property that would make it possible to use single shmem with two
mailboxes, one for rx and one for tx, but making sun6i mailbox have
bi-directional interface sounds easier.

regards,
	o.

>   git clone https://github.com/crust-firmware/meta meta
>   git clone -b mailbox-demo https://github.com/crust-firmware/crust meta/crust
>   cd meta
>   make
> 
> That will by default produce a U-Boot + ATF + SCP firmware image in
> [meta/]build/pinebook/u-boot-sunxi-with-spl.bin. See the top-level
> README.md for more information, such as cross-compiler setup.
> 
> I've now used this driver with three separate clients over the past two
> years, and they all work. If there are no remaining concerns with the
> driver, I'd like it to get merged.
> 
> Even without the driver, the clock patches (1-2) can go in at any time.
> 
> Changes from v3:
>   - Rebased on sunxi-next
>   - Added Rob's Reviewed-by for patch 3
>   - Fixed a crash when receiving a message on a disabled channel
>   - Cleaned up some comments/formatting in the driver
>   - Fixed #mbox-cells in sunxi-h3-h5.dtsi (patch 7)
>   - Removed the irqchip example (no longer relevant to the fw design)
>   - Added a demo/example client that uses the driver and a toy firmware
> 
> Changes from v2:
>   - Merge patches 1-3
>   - Add a comment in the code explaining the CLK_IS_CRITICAL usage
>   - Add a patch to mark the AR100 clocks as critical
>   - Use YAML for the device tree binding
>   - Include a not-for-merge example usage of the mailbox
> 
> Changes from v1:
>   - Marked message box clocks as critical instead of hacks in the driver
>   - 8 unidirectional channels instead of 4 bidirectional pairs
>   - Use per-SoC compatible strings and an A31 fallback compatible
>   - Dropped the mailbox framework patch
>   - Include DT patches for SoCs that document the message box
> 
> Samuel Holland (10):
>   clk: sunxi-ng: Mark msgbox clocks as critical
>   clk: sunxi-ng: Mark AR100 clocks as critical
>   dt-bindings: mailbox: Add a sunxi message box binding
>   mailbox: sunxi-msgbox: Add a new mailbox driver
>   ARM: dts: sunxi: a80: Add msgbox node
>   ARM: dts: sunxi: a83t: Add msgbox node
>   ARM: dts: sunxi: h3/h5: Add msgbox node
>   arm64: dts: allwinner: a64: Add msgbox node
>   arm64: dts: allwinner: h6: Add msgbox node
>   [DO NOT MERGE] drivers: firmware: msgbox demo
> 
>  .../mailbox/allwinner,sunxi-msgbox.yaml       |  79 +++++
>  arch/arm/boot/dts/sun8i-a83t.dtsi             |  10 +
>  arch/arm/boot/dts/sun9i-a80.dtsi              |  10 +
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi            |  10 +
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  34 ++
>  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  24 ++
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  10 +
>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c         |   3 +-
>  drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c        |   2 +-
>  drivers/clk/sunxi-ng/ccu-sun50i-h6.c          |   3 +-
>  drivers/clk/sunxi-ng/ccu-sun8i-a23.c          |   3 +-
>  drivers/clk/sunxi-ng/ccu-sun8i-a33.c          |   3 +-
>  drivers/clk/sunxi-ng/ccu-sun8i-a83t.c         |   3 +-
>  drivers/clk/sunxi-ng/ccu-sun8i-h3.c           |   3 +-
>  drivers/clk/sunxi-ng/ccu-sun8i-r.c            |   2 +-
>  drivers/clk/sunxi-ng/ccu-sun9i-a80.c          |   3 +-
>  drivers/firmware/Kconfig                      |   6 +
>  drivers/firmware/Makefile                     |   1 +
>  drivers/firmware/sunxi_msgbox_demo.c          | 307 +++++++++++++++++
>  drivers/mailbox/Kconfig                       |  10 +
>  drivers/mailbox/Makefile                      |   2 +
>  drivers/mailbox/sunxi-msgbox.c                | 323 ++++++++++++++++++
>  22 files changed, 842 insertions(+), 9 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
>  create mode 100644 drivers/firmware/sunxi_msgbox_demo.c
>  create mode 100644 drivers/mailbox/sunxi-msgbox.c
> 
> -- 
> 2.21.0
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 00/10] Allwinner sunxi message box support
  2019-09-09  3:22 ` [PATCH v4 00/10] Allwinner sunxi message box support Ondřej Jirman
@ 2019-09-09  3:54   ` Samuel Holland
  2019-09-09 12:36     ` Ondřej Jirman
  0 siblings, 1 reply; 29+ messages in thread
From: Samuel Holland @ 2019-09-09  3:54 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick, devicetree, linux-kernel, linux-sunxi,
	linux-clk, linux-arm-kernel

On 9/8/19 10:22 PM, Ondřej Jirman wrote:
> Hello Samuel,
> 
> On Mon, Aug 19, 2019 at 10:23:01PM -0500, Samuel Holland wrote:
>> This series adds support for the "hardware message box" in sun8i, sun9i,
>> and sun50i SoCs, used for communication with the ARISC management
>> processor (the platform's equivalent of the ARM SCP). The end goal is to
>> use the arm_scpi driver as a client, communicating with firmware running
>> on the AR100 CPU, or to use the mailbox to forward NMIs that the
>> firmware picks up from R_INTC.
>>
>> Unfortunately, the ARM SCPI client no longer works with this driver
>> since it now exposes all 8 hardware FIFOs individually. The SCPI client
>> could be made to work (and I posted proof-of-concept code to that effect
>> with v1 of this series), but that is a low priority, as Linux does not
>> directly use SCPI with the current firmware version; all SCPI use goes
>> through ATF via PSCI.
>>
>> As requested in the comments to v3 of this patchset, a demo client is
>> provided in the final patch. This demo goes along with a toy firmware
>> which shows that the driver does indeed work for two-way communication
>> on all channels. To build the firmware component, run:
> 
> I've tried using this driver with mainline arm_scpi driver (which is probably
> an expected future use, since crust provides SCPI interface).

If you've verified in some way that this driver works on A83T, I'd appreciate
your Tested-by, so I can send a patch for the A83T device tree node.

> The problem I've found is that arm_scpi expects message box to be
> bi-directional, but this driver provides uni-directional interface.
> 
> What do you think about making this driver provide bi-directional interface?
> We could halve the number of channels to 4 and mandate TX/RX configuration
> (from main CPU's PoV) as ABI.

Funny you mention that. That's what I did originally for v1, but it got NAKed by
Maxime, Andre, and Jassi:

https://lkml.org/lkml/2018/2/28/125
https://lkml.org/lkml/2018/2/28/944

> Otherwise it's impossible to use it with the arm_scpi driver.
> 
> Or do you have any other ideas? I guess arm_scpi can be fixed to add a
> property that would make it possible to use single shmem with two
> mailboxes, one for rx and one for tx, but making sun6i mailbox have
> bi-directional interface sounds easier.

Yes, you can use the existence of the mbox-names property to determine if the
driver needs one mailbox or two, as I did in this driver:

https://lkml.org/lkml/2019/3/1/789

I'll have a patch available soon that implements this for arm_scpi.

Cheers,
Samuel

> regards,
> 	o.
> 
>>   git clone https://github.com/crust-firmware/meta meta
>>   git clone -b mailbox-demo https://github.com/crust-firmware/crust meta/crust
>>   cd meta
>>   make
>>
>> That will by default produce a U-Boot + ATF + SCP firmware image in
>> [meta/]build/pinebook/u-boot-sunxi-with-spl.bin. See the top-level
>> README.md for more information, such as cross-compiler setup.
>>
>> I've now used this driver with three separate clients over the past two
>> years, and they all work. If there are no remaining concerns with the
>> driver, I'd like it to get merged.
>>
>> Even without the driver, the clock patches (1-2) can go in at any time.
>>
>> Changes from v3:
>>   - Rebased on sunxi-next
>>   - Added Rob's Reviewed-by for patch 3
>>   - Fixed a crash when receiving a message on a disabled channel
>>   - Cleaned up some comments/formatting in the driver
>>   - Fixed #mbox-cells in sunxi-h3-h5.dtsi (patch 7)
>>   - Removed the irqchip example (no longer relevant to the fw design)
>>   - Added a demo/example client that uses the driver and a toy firmware
>>
>> Changes from v2:
>>   - Merge patches 1-3
>>   - Add a comment in the code explaining the CLK_IS_CRITICAL usage
>>   - Add a patch to mark the AR100 clocks as critical
>>   - Use YAML for the device tree binding
>>   - Include a not-for-merge example usage of the mailbox
>>
>> Changes from v1:
>>   - Marked message box clocks as critical instead of hacks in the driver
>>   - 8 unidirectional channels instead of 4 bidirectional pairs
>>   - Use per-SoC compatible strings and an A31 fallback compatible
>>   - Dropped the mailbox framework patch
>>   - Include DT patches for SoCs that document the message box
>>
>> Samuel Holland (10):
>>   clk: sunxi-ng: Mark msgbox clocks as critical
>>   clk: sunxi-ng: Mark AR100 clocks as critical
>>   dt-bindings: mailbox: Add a sunxi message box binding
>>   mailbox: sunxi-msgbox: Add a new mailbox driver
>>   ARM: dts: sunxi: a80: Add msgbox node
>>   ARM: dts: sunxi: a83t: Add msgbox node
>>   ARM: dts: sunxi: h3/h5: Add msgbox node
>>   arm64: dts: allwinner: a64: Add msgbox node
>>   arm64: dts: allwinner: h6: Add msgbox node
>>   [DO NOT MERGE] drivers: firmware: msgbox demo
>>
>>  .../mailbox/allwinner,sunxi-msgbox.yaml       |  79 +++++
>>  arch/arm/boot/dts/sun8i-a83t.dtsi             |  10 +
>>  arch/arm/boot/dts/sun9i-a80.dtsi              |  10 +
>>  arch/arm/boot/dts/sunxi-h3-h5.dtsi            |  10 +
>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  34 ++
>>  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  24 ++
>>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  10 +
>>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c         |   3 +-
>>  drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c        |   2 +-
>>  drivers/clk/sunxi-ng/ccu-sun50i-h6.c          |   3 +-
>>  drivers/clk/sunxi-ng/ccu-sun8i-a23.c          |   3 +-
>>  drivers/clk/sunxi-ng/ccu-sun8i-a33.c          |   3 +-
>>  drivers/clk/sunxi-ng/ccu-sun8i-a83t.c         |   3 +-
>>  drivers/clk/sunxi-ng/ccu-sun8i-h3.c           |   3 +-
>>  drivers/clk/sunxi-ng/ccu-sun8i-r.c            |   2 +-
>>  drivers/clk/sunxi-ng/ccu-sun9i-a80.c          |   3 +-
>>  drivers/firmware/Kconfig                      |   6 +
>>  drivers/firmware/Makefile                     |   1 +
>>  drivers/firmware/sunxi_msgbox_demo.c          | 307 +++++++++++++++++
>>  drivers/mailbox/Kconfig                       |  10 +
>>  drivers/mailbox/Makefile                      |   2 +
>>  drivers/mailbox/sunxi-msgbox.c                | 323 ++++++++++++++++++
>>  22 files changed, 842 insertions(+), 9 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
>>  create mode 100644 drivers/firmware/sunxi_msgbox_demo.c
>>  create mode 100644 drivers/mailbox/sunxi-msgbox.c
>>
>> -- 
>> 2.21.0
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 00/10] Allwinner sunxi message box support
  2019-09-09  3:54   ` Samuel Holland
@ 2019-09-09 12:36     ` Ondřej Jirman
  0 siblings, 0 replies; 29+ messages in thread
From: Ondřej Jirman @ 2019-09-09 12:36 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Maxime Ripard, Chen-Yu Tsai, Jassi Brar, Michael Turquette,
	Stephen Boyd, Rob Herring, Mark Rutland, Corentin Labbe,
	Vasily Khoruzhick, devicetree, linux-kernel, linux-sunxi,
	linux-clk, linux-arm-kernel

Hi,

On Sun, Sep 08, 2019 at 10:54:17PM -0500, Samuel Holland wrote:
> On 9/8/19 10:22 PM, Ondřej Jirman wrote:
> > Hello Samuel,
> > 
> > On Mon, Aug 19, 2019 at 10:23:01PM -0500, Samuel Holland wrote:
> >> This series adds support for the "hardware message box" in sun8i, sun9i,
> >> and sun50i SoCs, used for communication with the ARISC management
> >> processor (the platform's equivalent of the ARM SCP). The end goal is to
> >> use the arm_scpi driver as a client, communicating with firmware running
> >> on the AR100 CPU, or to use the mailbox to forward NMIs that the
> >> firmware picks up from R_INTC.
> >>
> >> Unfortunately, the ARM SCPI client no longer works with this driver
> >> since it now exposes all 8 hardware FIFOs individually. The SCPI client
> >> could be made to work (and I posted proof-of-concept code to that effect
> >> with v1 of this series), but that is a low priority, as Linux does not
> >> directly use SCPI with the current firmware version; all SCPI use goes
> >> through ATF via PSCI.
> >>
> >> As requested in the comments to v3 of this patchset, a demo client is
> >> provided in the final patch. This demo goes along with a toy firmware
> >> which shows that the driver does indeed work for two-way communication
> >> on all channels. To build the firmware component, run:
> > 
> > I've tried using this driver with mainline arm_scpi driver (which is probably
> > an expected future use, since crust provides SCPI interface).
> 
> If you've verified in some way that this driver works on A83T, I'd appreciate
> your Tested-by, so I can send a patch for the A83T device tree node.

Tested-by: Ondrej Jirman <megous@megous.com>

(on A83T)

> > The problem I've found is that arm_scpi expects message box to be
> > bi-directional, but this driver provides uni-directional interface.
> > 
> > What do you think about making this driver provide bi-directional interface?
> > We could halve the number of channels to 4 and mandate TX/RX configuration
> > (from main CPU's PoV) as ABI.
> 
> Funny you mention that. That's what I did originally for v1, but it got NAKed by
> Maxime, Andre, and Jassi:
> 
> https://lkml.org/lkml/2018/2/28/125
> https://lkml.org/lkml/2018/2/28/944
> 
> > Otherwise it's impossible to use it with the arm_scpi driver.
> > 
> > Or do you have any other ideas? I guess arm_scpi can be fixed to add a
> > property that would make it possible to use single shmem with two
> > mailboxes, one for rx and one for tx, but making sun6i mailbox have
> > bi-directional interface sounds easier.
> 
> Yes, you can use the existence of the mbox-names property to determine if the
> driver needs one mailbox or two, as I did in this driver:
> 
> https://lkml.org/lkml/2019/3/1/789
> 
> I'll have a patch available soon that implements this for arm_scpi.

Yeah, I've patched arm_scpi too. :)

https://megous.com/git/linux/commit/?h=tbs-5.3&id=69a0cd0093a63039ace2f763e8d82009c50ff03c

(but that's just for the test, because it breaks the existing interface for
other uses)

Anyway, using mbox-names looks like a nice solution! Thanks! Though,
arm_scpi driver has a bit more complicated existing interface, where it can use
multiple mailboxes and rotates through them after every message.

BTW, I'm slowly laboring through understanding how to get suspend to ram working
on one A83T tablet. https://xnux.eu/tablet-hacking/ Which is how I tested this
driver.

regards,
	o.

> Cheers,
> Samuel
> 
> > regards,
> > 	o.
> > 
> >>   git clone https://github.com/crust-firmware/meta meta
> >>   git clone -b mailbox-demo https://github.com/crust-firmware/crust meta/crust
> >>   cd meta
> >>   make
> >>
> >> That will by default produce a U-Boot + ATF + SCP firmware image in
> >> [meta/]build/pinebook/u-boot-sunxi-with-spl.bin. See the top-level
> >> README.md for more information, such as cross-compiler setup.
> >>
> >> I've now used this driver with three separate clients over the past two
> >> years, and they all work. If there are no remaining concerns with the
> >> driver, I'd like it to get merged.
> >>
> >> Even without the driver, the clock patches (1-2) can go in at any time.
> >>
> >> Changes from v3:
> >>   - Rebased on sunxi-next
> >>   - Added Rob's Reviewed-by for patch 3
> >>   - Fixed a crash when receiving a message on a disabled channel
> >>   - Cleaned up some comments/formatting in the driver
> >>   - Fixed #mbox-cells in sunxi-h3-h5.dtsi (patch 7)
> >>   - Removed the irqchip example (no longer relevant to the fw design)
> >>   - Added a demo/example client that uses the driver and a toy firmware
> >>
> >> Changes from v2:
> >>   - Merge patches 1-3
> >>   - Add a comment in the code explaining the CLK_IS_CRITICAL usage
> >>   - Add a patch to mark the AR100 clocks as critical
> >>   - Use YAML for the device tree binding
> >>   - Include a not-for-merge example usage of the mailbox
> >>
> >> Changes from v1:
> >>   - Marked message box clocks as critical instead of hacks in the driver
> >>   - 8 unidirectional channels instead of 4 bidirectional pairs
> >>   - Use per-SoC compatible strings and an A31 fallback compatible
> >>   - Dropped the mailbox framework patch
> >>   - Include DT patches for SoCs that document the message box
> >>
> >> Samuel Holland (10):
> >>   clk: sunxi-ng: Mark msgbox clocks as critical
> >>   clk: sunxi-ng: Mark AR100 clocks as critical
> >>   dt-bindings: mailbox: Add a sunxi message box binding
> >>   mailbox: sunxi-msgbox: Add a new mailbox driver
> >>   ARM: dts: sunxi: a80: Add msgbox node
> >>   ARM: dts: sunxi: a83t: Add msgbox node
> >>   ARM: dts: sunxi: h3/h5: Add msgbox node
> >>   arm64: dts: allwinner: a64: Add msgbox node
> >>   arm64: dts: allwinner: h6: Add msgbox node
> >>   [DO NOT MERGE] drivers: firmware: msgbox demo
> >>
> >>  .../mailbox/allwinner,sunxi-msgbox.yaml       |  79 +++++
> >>  arch/arm/boot/dts/sun8i-a83t.dtsi             |  10 +
> >>  arch/arm/boot/dts/sun9i-a80.dtsi              |  10 +
> >>  arch/arm/boot/dts/sunxi-h3-h5.dtsi            |  10 +
> >>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  34 ++
> >>  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi  |  24 ++
> >>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  10 +
> >>  drivers/clk/sunxi-ng/ccu-sun50i-a64.c         |   3 +-
> >>  drivers/clk/sunxi-ng/ccu-sun50i-h6-r.c        |   2 +-
> >>  drivers/clk/sunxi-ng/ccu-sun50i-h6.c          |   3 +-
> >>  drivers/clk/sunxi-ng/ccu-sun8i-a23.c          |   3 +-
> >>  drivers/clk/sunxi-ng/ccu-sun8i-a33.c          |   3 +-
> >>  drivers/clk/sunxi-ng/ccu-sun8i-a83t.c         |   3 +-
> >>  drivers/clk/sunxi-ng/ccu-sun8i-h3.c           |   3 +-
> >>  drivers/clk/sunxi-ng/ccu-sun8i-r.c            |   2 +-
> >>  drivers/clk/sunxi-ng/ccu-sun9i-a80.c          |   3 +-
> >>  drivers/firmware/Kconfig                      |   6 +
> >>  drivers/firmware/Makefile                     |   1 +
> >>  drivers/firmware/sunxi_msgbox_demo.c          | 307 +++++++++++++++++
> >>  drivers/mailbox/Kconfig                       |  10 +
> >>  drivers/mailbox/Makefile                      |   2 +
> >>  drivers/mailbox/sunxi-msgbox.c                | 323 ++++++++++++++++++
> >>  22 files changed, 842 insertions(+), 9 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sunxi-msgbox.yaml
> >>  create mode 100644 drivers/firmware/sunxi_msgbox_demo.c
> >>  create mode 100644 drivers/mailbox/sunxi-msgbox.c
> >>
> >> -- 
> >> 2.21.0
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2019-09-09 12:36 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-20  3:23 [PATCH v4 00/10] Allwinner sunxi message box support Samuel Holland
2019-08-20  3:23 ` [PATCH v4 01/10] clk: sunxi-ng: Mark msgbox clocks as critical Samuel Holland
2019-08-20  3:23 ` [PATCH v4 02/10] clk: sunxi-ng: Mark AR100 " Samuel Holland
2019-08-20  7:11   ` Maxime Ripard
2019-08-20 13:02     ` Samuel Holland
2019-08-21 12:24       ` Maxime Ripard
2019-09-05 18:56         ` Stephen Boyd
2019-08-20  3:23 ` [PATCH v4 03/10] dt-bindings: mailbox: Add a sunxi message box binding Samuel Holland
2019-08-20  7:14   ` Maxime Ripard
2019-08-20 13:04     ` Samuel Holland
2019-08-21 12:07       ` Maxime Ripard
2019-08-20  3:23 ` [PATCH v4 04/10] mailbox: sunxi-msgbox: Add a new mailbox driver Samuel Holland
2019-08-20  8:27   ` Maxime Ripard
2019-08-20 11:18   ` Ondřej Jirman
2019-08-20 13:07     ` Samuel Holland
2019-08-20 13:34       ` Ondřej Jirman
2019-08-21 12:30       ` Maxime Ripard
2019-08-20  3:23 ` [PATCH v4 05/10] ARM: dts: sunxi: a80: Add msgbox node Samuel Holland
2019-08-20  8:15   ` Maxime Ripard
2019-08-20 13:17     ` Samuel Holland
2019-08-23 14:56       ` Maxime Ripard
2019-08-20  3:23 ` [PATCH v4 06/10] ARM: dts: sunxi: a83t: " Samuel Holland
2019-08-20  3:23 ` [PATCH v4 07/10] ARM: dts: sunxi: h3/h5: " Samuel Holland
2019-08-20  3:23 ` [PATCH v4 08/10] arm64: dts: allwinner: a64: " Samuel Holland
2019-08-20  3:23 ` [PATCH v4 09/10] arm64: dts: allwinner: h6: " Samuel Holland
2019-08-20  3:23 ` [PATCH v4 10/10] [DO NOT MERGE] drivers: firmware: msgbox demo Samuel Holland
2019-09-09  3:22 ` [PATCH v4 00/10] Allwinner sunxi message box support Ondřej Jirman
2019-09-09  3:54   ` Samuel Holland
2019-09-09 12:36     ` Ondřej Jirman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).