[PATCH v3 0/2] hwspinlock: add sun8i hardware spinlock support

[PATCH v3 0/2] hwspinlock: add sun8i hardware spinlock support

[Wilken Gottwalt]

Most of the Allwinner sun8i compatible devices contain a spinlock unit
which can be used to sync access to devices shared between the ARM cores
and the embedded OpenRisc AR100 core. According to the datasheets at
least 32 spinlocks are supported. The implementation supports 32, 64,
128 and 256 spinlock setups, but there is no known SoC yet, which
implements more than 32 spinlocks.

This driver adds support for this hardware spinlock unit to Linux
including all 4 possible setups. The driver reports the found setup via
debugfs. It can be build as a builtin and normal module by using the
HWSPINLOCK_SUN8I symbol.

This driver is the first step to enable hwspinlock support in Linux, but
also requires support in the firmware of the OpenRisc core. This patch
provides the driver and binding documentation but is not yet included
into the sun8i and sun50i dtsi files. Also not every sun8i or sun50i SoC
seem to have support for this hardware. For example the H616 is missing
the whole spinlock section in the datasheets.

The spinlock hardware has two ways to figure out if a lock is taken. The
lock can simply be read or bits of a 32bit wide status register can be
checked. The status register only supports the first 32 locks and may
not cover bigger spinlock setups. Therefore reading/writing a specific
spinlock for checking is used in the driver.

The status register is now free for debugging/testing purposes and can
completely bypass the Linux hwspinlock ABI. This status register will be
used in some additional kernel modules to test the hwspinlock driver.



Testing the driver.

To run all tests it is necessary to take locks on the OpenRisc core and
show on the Linux side that the locks were taken by an external event.
This can be achived by using the crust firmware. For this the crust
firmware needs to be changed to take and release spinlocks (a simple
MMIO operation on the hwlock registers), which is currently not
supported by the current crust firmware. The necessary crust fork can
be found here https://github.com/wgottwalt/crust (hwspinlock branch).
It is also necessary to build u-boot with support for this crust/SCP
firmware. This u-boot fork can be found here
https://github.com/crust-firmware/u-boot (crust branch).

For testing this driver it is also necessary to pick a device that is
fully supported by the crust firmware. For this the H5 based Friendlyarm
NanoPi NEO2 was used, which is fully supported by u-boot (and the fork),
the curst firmware (via H5 target) and current Linux kernels. In the
crust fork it is necessary to go into debug menu of "make nconfig" and
select the hwspinlock test loop. This debug option enables a loop that
goes through the first 32 spinlocks. It takes/releases a lock one after
another using the timeout functions (and hw timers) of the crust
firmware. A timeout can be set in the debug menu.



Test 1:

This test was done using a mainline u-boot and a crust enabled u-boot.
For this a simple second kernel module was used, which can be found here
https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test.

Using mainline u-boot it shows that the Linux side correctly takes a
lock, tries to recursively take a lock again (which does not happen) and
releases a lock. This is done for all 32 locks several times.

[   50.332836] [init]--- SUN8I HWSPINLOCK DRIVER TEST ---
[   50.338155] [run ]--- testing locks 0 to 31 ---
[   50.342725] [test] testing lock 0
[   50.346075] [test]+++ attempt #0 succeded
[   50.350103] [test]+++ attempt #1 succeded
[   50.354127] [test]+++ attempt #2 succeded
[   50.358154] [test] testing lock 1
[   50.361474] [test]+++ attempt #0 succeded
[   50.365496] [test]+++ attempt #1 succeded
[   50.369509] [test]+++ attempt #2 succeded
...
[   50.819369] [test] testing lock 31
[   50.822777] [test]+++ attempt #0 succeded
[   50.826796] [test]+++ attempt #1 succeded
[   50.830816] [test]+++ attempt #2 succeded

If the same test is done with the hwspinlock loop enabled crust firmware
and the crust enabled u-boot fork, the Linux test kernel module hits the
one lock taken by the crust firmware.

[  198.232316] [init]--- SUN8I HWSPINLOCK DRIVER TEST ---
[  198.237616] [run ]--- testing locks 0 to 31 ---
[  198.242170] [test] testing lock 0
[  198.245493] [test]+++ attempt #0 succeded
[  198.249514] [test]+++ attempt #1 succeded
[  198.253528] [test]+++ attempt #2 succeded
...
[  198.550564] [test] testing lock 20
[  198.553972] [test]+++ attempt #0 succeded
[  198.557983] [test]+++ attempt #1 succeded
[  198.561998] [test]+++ attempt #2 succeded
[  198.566018] [test] testing lock 21
[  198.569432] [test] taking lock attempt #0 failed (-16)
[  198.574580] [run ]--- testing specific lock 21 failed (-14) ---
[  198.580505] [test] testing lock 22
[  198.583918] [test]+++ attempt #0 succeded
[  198.587935] [test]+++ attempt #1 succeded
[  198.591954] [test]+++ attempt #2 succeded
...
[  198.719650] [test] testing lock 31
[  198.723065] [test]+++ attempt #0 succeded
[  198.727088] [test]+++ attempt #1 succeded
[  198.731099] [test]+++ attempt #2 succeded



Test 2:

This is a more complex test which uses the status register to bypass the
Linux hwspinlock ABI. For this to work a slightly modified driver is
used, which can be found here
https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/modified.
This modified driver splits the 4K memory range into two and leaves the
status register untouched, so that it can be used by another test kernel
modulei, which can be found here
https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test2.
It is also necessary to change the device tree entries to get both
kernel modules working in parallel.

hwspinlock-mod@1c18000 {
        compatible = "allwinner,sun8i-hwspinlock-mod";
        reg = <0x01c18000 0x4 0x01c18100 0x400>;
        clocks = <&ccu CLK_BUS_SPINLOCK>;
        clock-names = "ahb";
        resets = <&ccu RST_BUS_SPINLOCK>;
        reset-names = "ahb";
        status = "okay";
};

hwspinlock-stat@1c18010 {
        compatible = "allwinner,sun8i-hwspinlock-stat";
        reg = <0x01c18010 0x4>;
        status = "okay";
};

The extended test kernel module supports four different modes to test
the hwspinlocks. Two of them are sufficient to show the spinlock
mechanism working.

Test 2 Mode 1:

This test reads and prints the status register continuously. The crust
firmware and the test are set to a hwspinlock timeout of one second. The
test kernel module code runs a bit slower because of more code executed
and all the printing. Because of that you can see how one lock is missed
completely between entry 8 and 9.

# modprobe sun8i_hwspinlock_test2 mode=1 loops=10
[  182.685525] [init]--- SUN8I HWSPINLOCK DRIVER ENHANCED TEST ---
[  182.691882] [sreg] 00000100_00000000_00000000_00000000
[  183.715763] [sreg] 00000010_00000000_00000000_00000000
[  184.739777] [sreg] 00000001_00000000_00000000_00000000
[  185.763761] [sreg] 00000000_10000000_00000000_00000000
[  186.787760] [sreg] 00000000_01000000_00000000_00000000
[  187.811761] [sreg] 00000000_00100000_00000000_00000000
[  188.835763] [sreg] 00000000_00010000_00000000_00000000
[  189.859761] [sreg] 00000000_00001000_00000000_00000000
[  190.883760] [sreg] 00000000_00000010_00000000_00000000
[  191.907760] [sreg] 00000000_00000001_00000000_00000000

Test 2 Mode 3:

This test combines the Linux hwspinlock ABI approach from the first test
and the status register access. The "after" reads show the locks taken
by both sides until the Linux hwspinlock driver tries to take the lock
taken by the crust firmware.

[  315.452080] [test]+++ attempt #0 succeded
[  315.456106] [sreg] before take 00000000_00000100_00000000_00000000
[  315.462292] [sreg] after take 00000000_00001100_00000000_00000000
[  315.468399] [sreg] after recursive take 00000000_00001100_00000000_00000000
[  315.475368] [sreg] after untake 00000000_00000100_00000000_00000000
[  315.481643] [test]+++ attempt #1 succeded
[  315.485664] [sreg] before take 00000000_00000100_00000000_00000000
[  315.491856] [sreg] after take 00000000_00001100_00000000_00000000
[  315.497958] [sreg] after recursive take 00000000_00001100_00000000_00000000
[  315.504938] [sreg] after untake 00000000_00000100_00000000_00000000
[  315.511205] [test]+++ attempt #2 succeded
[  315.515225] [test] testing lock 13
[  315.518642] [sreg] before take 00000000_00000100_00000000_00000000
[  315.524830] [test] taking lock attempt #0 failed (-16)
[  315.529972] [run ]--- testing specific lock 13 failed (-14) ---
[  315.535898] [test] testing lock 14
[  315.539305] [sreg] before take 00000000_00000100_00000000_00000000
[  315.545498] [sreg] after take 00000000_00000110_00000000_00000000
[  315.551599] [sreg] after recursive take 00000000_00000110_00000000_00000000
[  315.558573] [sreg] after untake 00000000_00000100_00000000_00000000
[  315.564844] [test]+++ attempt #0 succeded
[  315.568870] [sreg] before take 00000000_00000100_00000000_00000000
[  315.575058] [sreg] after take 00000000_00000110_00000000_00000000
[  315.581169] [sreg] after recursive take 00000000_00000110_00000000_00000000
[  315.588138] [sreg] after untake 00000000_00000100_00000000_00000000
[  315.594409] [test]+++ attempt #1 succeded



This patch adds:
- hwspinlock driver suni8i_hwspinlock
- updates makefiles
- hwspinlock dt bindings documentation
- updates MAINTAINERS

Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>

Changes in v3:
  - moved test description to cover letter
  - changed name and symbols from sunxi to sun8i
  - improved driver description
  - further simpliefied driver
  - fully switched to devm_* and devm_add_action_* functions

Changes in v2:
  - redone coverletter
  - fixed ranges in the device tree description
  - added suggestions from Bjorn Andersson and Maxime Ripard to the driver
  - provided better driver and test description

Wilken Gottwalt (2):
  dt-bindings: hwlock: add sun8i_hwspinlock documentation
  hwspinlock: add sun8i hardware spinlock support

 .../bindings/hwlock/sun8i-hwspinlock.yaml     |  63 ++++++
 MAINTAINERS                                   |   6 +
 drivers/hwspinlock/Kconfig                    |   9 +
 drivers/hwspinlock/Makefile                   |   1 +
 drivers/hwspinlock/sun8i_hwspinlock.c         | 205 ++++++++++++++++++
 5 files changed, 284 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
 create mode 100644 drivers/hwspinlock/sun8i_hwspinlock.c

-- 
2.29.2

[PATCH v3 1/2] dt-bindings: hwlock: add sun8i_hwspinlock documentation

[Wilken Gottwalt]

Adds documentation on how to use the sun8i_hwspinlock driver for sun8i
compatible SoCs.

Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
---
 .../bindings/hwlock/sun8i-hwspinlock.yaml     | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml

diff --git a/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
new file mode 100644
index 000000000000..2954ee0b36a7
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwlock/sun8i-hwspinlock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SUN8I hardware spinlock driver for Allwinner sun8i compatible SoCs
+
+maintainers:
+  - Wilken Gottwalt <wilken.gottwalt@posteo.net>
+
+properties:
+  compatible:
+    enum:
+      - allwinner,sun8i-hwspinlock
+
+    reg:          # 0x01C18000 (H2+, H3, H5), 0x03004000 (H6), length 0x1000
+      maxItems: 1
+
+    clocks:       # phandle to the reference clock
+      maxItems: 1
+
+    clock-names:  # name of the bus ("ahb")
+      maxItems: 1
+
+    resets:       # phandle to the reset control
+      maxItems: 1
+
+    reset-names:  # name of the bus ("ahb")
+      maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+
+additionalProperties: false
+
+examples:
+
+  - |
+    /* H2+ based OrangePi Zero */
+    hwspinlock: hwspinlock@1C18000 {
+      compatible = "allwinner,sun8i-hwspinlock";
+      reg = <0x01c18000 0x1000>;
+      clocks = <&ccu CLK_BUS_SPINLOCK>;
+      clock-names = "ahb";
+      resets = <&ccu RST_BUS_SPINLOCK>;
+      reset-names = "ahb";
+    };
+
+    /* H6 based OrangePi 3 */
+    hwspinlock: hwspinlock@3004000 {
+      compatible = "allwinner,sun8i-hwspinlock";
+      reg = <0x03004000 0x1000>;
+      clocks = <&ccu CLK_BUS_SPINLOCK>;
+      clock-names = "ahb";
+      resets = <&ccu RST_BUS_SPINLOCK>;
+      reset-names = "ahb";
+    };
-- 
2.29.2

[PATCH v3 2/2] hwspinlock: add sun8i hardware spinlock support

[Wilken Gottwalt]

Adds the sun8i_hwspinlock driver for the hardware spinlock unit found in
most of the sun8i compatible SoCs.

This unit provides at least 32 spinlocks in hardware. The implementation
supports 32, 64, 128 or 256 32bit registers. A lock can be taken by
reading a register and released by writing a 0 to it. This driver
supports all 4 spinlock setups, but for now only the first setup (32
locks) seem to exist in available devices. This spinlock unit is shared
between all ARM cores and the embedded OpenRisc AR100 core. All of them
can take/release a lock with a single cycle operation. It can be used to
sync access to devices shared by the ARM cores and the OpenRisc core.

There are two ways to check if a lock is taken. The first way is to read
a lock. If a 0 is returned, the lock was free and is taken now. If an 1
is returned, the caller has to try again. Which means the lock is taken.
The second way is to read a 32bit wide status register where every bit
represents one of the 32 first locks. According to the datasheets this
status register supports only the 32 first locks. This is the reason the
first way (lock read/write) approach is used to be able to cover all 256
locks in future devices. The driver also reports the amount of supported
locks via debugfs.

Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
---
Changes in v3:
  - moved test description to cover letter
  - changed name and symbols from sunxi to sun8i
  - improved driver description
  - further simpliefied driver
  - fully switched to devm_* and devm_add_action_* functions

Changes in v2:
  - added suggestions from Bjorn Andersson and Maxime Ripard
  - provided better driver and test description
---
 MAINTAINERS                           |   6 +
 drivers/hwspinlock/Kconfig            |   9 ++
 drivers/hwspinlock/Makefile           |   1 +
 drivers/hwspinlock/sun8i_hwspinlock.c | 205 ++++++++++++++++++++++++++
 4 files changed, 221 insertions(+)
 create mode 100644 drivers/hwspinlock/sun8i_hwspinlock.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ebe4829cdd4d..46846113f1eb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -722,6 +722,12 @@ L:	linux-crypto@vger.kernel.org
 S:	Maintained
 F:	drivers/crypto/allwinner/
 
+ALLWINNER HARDWARE SPINLOCK SUPPORT
+M:	Wilken Gottwalt <wilken.gottwalt@posteo.net>
+S:	Maintained
+F:	Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
+F:	drivers/hwspinlock/sun8i_hwspinlock.c
+
 ALLWINNER THERMAL DRIVER
 M:	Vasily Khoruzhick <anarsoul@gmail.com>
 M:	Yangtao Li <tiny.windzz@gmail.com>
diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig
index 32cd26352f38..b03fd99aab32 100644
--- a/drivers/hwspinlock/Kconfig
+++ b/drivers/hwspinlock/Kconfig
@@ -55,6 +55,15 @@ config HWSPINLOCK_STM32
 
 	  If unsure, say N.
 
+config HWSPINLOCK_SUN8I
+	tristate "SUN8I Hardware Spinlock device"
+	depends on ARCH_SUNXI || COMPILE_TEST
+	help
+	  Say y here to support the SUN8I Hardware Spinlock device which can be
+	  found in most of the sun8i compatible Allwinner SoCs.
+
+	  If unsure, say N.
+
 config HSEM_U8500
 	tristate "STE Hardware Semaphore functionality"
 	depends on ARCH_U8500 || COMPILE_TEST
diff --git a/drivers/hwspinlock/Makefile b/drivers/hwspinlock/Makefile
index ed053e3f02be..3496648d9257 100644
--- a/drivers/hwspinlock/Makefile
+++ b/drivers/hwspinlock/Makefile
@@ -9,4 +9,5 @@ obj-$(CONFIG_HWSPINLOCK_QCOM)		+= qcom_hwspinlock.o
 obj-$(CONFIG_HWSPINLOCK_SIRF)		+= sirf_hwspinlock.o
 obj-$(CONFIG_HWSPINLOCK_SPRD)		+= sprd_hwspinlock.o
 obj-$(CONFIG_HWSPINLOCK_STM32)		+= stm32_hwspinlock.o
+obj-$(CONFIG_HWSPINLOCK_SUN8I)		+= sun8i_hwspinlock.o
 obj-$(CONFIG_HSEM_U8500)		+= u8500_hsem.o
diff --git a/drivers/hwspinlock/sun8i_hwspinlock.c b/drivers/hwspinlock/sun8i_hwspinlock.c
new file mode 100644
index 000000000000..0a48a7c1365a
--- /dev/null
+++ b/drivers/hwspinlock/sun8i_hwspinlock.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * sun8i_hwspinlock.c - hardware spinlock driver for sun8i compatible Allwinner SoCs
+ * Copyright (C) 2020 Wilken Gottwalt <wilken.gottwalt@posteo.net>
+ */
+
+#include <linux/clk.h>
+#include <linux/debugfs.h>
+#include <linux/errno.h>
+#include <linux/hwspinlock.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#include "hwspinlock_internal.h"
+
+#define DRIVER_NAME		"sun8i_hwspinlock"
+
+#define SPINLOCK_BASE_ID	0 /* there is only one hwspinlock device per SoC */
+#define SPINLOCK_SYSSTATUS_REG	0x0000
+#define SPINLOCK_LOCK_REGN	0x0100
+#define SPINLOCK_NOTTAKEN	0
+
+struct sun8i_hwspinlock_data {
+	struct hwspinlock_device *bank;
+	struct reset_control *reset;
+	struct clk *ahb_clk;
+	struct dentry *debugfs;
+	int nlocks;
+};
+
+#ifdef CONFIG_DEBUG_FS
+
+static int hwlocks_supported_show(struct seq_file *seqf, void *unused)
+{
+	struct sun8i_hwspinlock_data *priv = seqf->private;
+
+	seq_printf(seqf, "%d\n", priv->nlocks);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(hwlocks_supported);
+
+static void sun8i_hwspinlock_debugfs_init(struct sun8i_hwspinlock_data *priv)
+{
+	priv->debugfs = debugfs_create_dir(DRIVER_NAME, NULL);
+	debugfs_create_file("supported", 0444, priv->debugfs, priv, &hwlocks_supported_fops);
+}
+
+#else
+
+static void sun8i_hwspinlock_debugfs_init(struct sun8i_hwspinlock_data *priv)
+{
+}
+
+#endif
+
+static int sun8i_hwspinlock_trylock(struct hwspinlock *lock)
+{
+	void __iomem *lock_addr = lock->priv;
+
+	return (readl(lock_addr) == SPINLOCK_NOTTAKEN);
+}
+
+static void sun8i_hwspinlock_unlock(struct hwspinlock *lock)
+{
+	void __iomem *lock_addr = lock->priv;
+
+	writel(SPINLOCK_NOTTAKEN, lock_addr);
+}
+
+static const struct hwspinlock_ops sun8i_hwspinlock_ops = {
+	.trylock	= sun8i_hwspinlock_trylock,
+	.unlock		= sun8i_hwspinlock_unlock,
+};
+
+static void sun8i_hwspinlock_disable(void *data)
+{
+	struct sun8i_hwspinlock_data *priv = data;
+
+	debugfs_remove_recursive(priv->debugfs);
+	reset_control_assert(priv->reset);
+	clk_disable_unprepare(priv->ahb_clk);
+}
+
+static int sun8i_hwspinlock_probe(struct platform_device *pdev)
+{
+	struct sun8i_hwspinlock_data *priv;
+	struct hwspinlock *hwlock;
+	void __iomem *io_base;
+	u32 num_banks;
+	int err, i;
+
+	io_base = devm_platform_ioremap_resource(pdev, SPINLOCK_BASE_ID);
+	if (IS_ERR(io_base)) {
+		err = PTR_ERR(io_base);
+		dev_err(&pdev->dev, "unable to request MMIO (%d)\n", err);
+		return err;
+	}
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	err = devm_add_action_or_reset(&pdev->dev, sun8i_hwspinlock_disable, priv);
+	if (err) {
+		dev_err(&pdev->dev, "unable to add disable action\n");
+		return err;
+	}
+
+	priv->ahb_clk = devm_clk_get(&pdev->dev, "ahb");
+	if (IS_ERR(priv->ahb_clk)) {
+		err = PTR_ERR(priv->ahb_clk);
+		dev_err(&pdev->dev, "unable to get AHB clock (%d)\n", err);
+		return err;
+	}
+
+	priv->reset = devm_reset_control_get_optional(&pdev->dev, "ahb");
+	if (IS_ERR(priv->reset)) {
+		return dev_err_probe(&pdev->dev, PTR_ERR(priv->reset),
+				     "unable to get reset control\n");
+	}
+
+	err = reset_control_deassert(priv->reset);
+	if (err) {
+		dev_err(&pdev->dev, "deassert reset control failure (%d)\n", err);
+		return err;
+	}
+
+	err = clk_prepare_enable(priv->ahb_clk);
+	if (err) {
+		dev_err(&pdev->dev, "unable to prepare AHB clk (%d)\n", err);
+		return err;
+	}
+
+	/*
+	 * bit 28 and 29 hold the amount of spinlock banks, but at the same time the datasheet
+	 * says, bit 30 and 31 are reserved while the values can be 0 to 4, which is not reachable
+	 * by two bits alone, so the reserved bits are also taken into account
+	 */
+	num_banks = readl(io_base + SPINLOCK_SYSSTATUS_REG) >> 28;
+	switch (num_banks) {
+	case 1 ... 4:
+		/*
+		 * 32, 64, 128 and 256 spinlocks are supported by the hardware implementation,
+		 * though most of the SoCs support 32 spinlocks only
+		 */
+		priv->nlocks = 1 << (4 + num_banks);
+		break;
+	default:
+		dev_err(&pdev->dev, "unsupported hwspinlock setup (%d)\n", num_banks);
+		return -EINVAL;
+	}
+
+	priv->bank = devm_kzalloc(&pdev->dev, struct_size(priv->bank, lock, priv->nlocks),
+				  GFP_KERNEL);
+	if (!priv->bank)
+		return -ENOMEM;
+
+	for (i = 0; i < priv->nlocks; ++i) {
+		hwlock = &priv->bank->lock[i];
+		hwlock->priv = io_base + SPINLOCK_LOCK_REGN + sizeof(u32) * i;
+	}
+
+	sun8i_hwspinlock_debugfs_init(priv);
+	platform_set_drvdata(pdev, priv);
+
+	return devm_hwspin_lock_register(&pdev->dev, priv->bank, &sun8i_hwspinlock_ops,
+					 SPINLOCK_BASE_ID, priv->nlocks);
+}
+
+static const struct of_device_id sun8i_hwspinlock_ids[] = {
+	{ .compatible = "allwinner,sun8i-hwspinlock", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sun8i_hwspinlock_ids);
+
+static struct platform_driver sun8i_hwspinlock_driver = {
+	.probe	= sun8i_hwspinlock_probe,
+	.driver	= {
+		.name		= DRIVER_NAME,
+		.of_match_table	= sun8i_hwspinlock_ids,
+	},
+};
+
+static int __init sun8i_hwspinlock_init(void)
+{
+	return platform_driver_register(&sun8i_hwspinlock_driver);
+}
+module_init(sun8i_hwspinlock_init);
+
+static void __exit sun8i_hwspinlock_exit(void)
+{
+	platform_driver_unregister(&sun8i_hwspinlock_driver);
+}
+module_exit(sun8i_hwspinlock_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("SUN8I hardware spinlock driver");
+MODULE_AUTHOR("Wilken Gottwalt <wilken.gottwalt@posteo.net>");
-- 
2.29.2

Re: [PATCH v3 1/2] dt-bindings: hwlock: add sun8i_hwspinlock documentation

[Maxime Ripard]

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

Hi,

On Mon, Dec 07, 2020 at 05:05:03PM +0100, Wilken Gottwalt wrote:
> Adds documentation on how to use the sun8i_hwspinlock driver for sun8i
> compatible SoCs.
> 
> Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> ---
>  .../bindings/hwlock/sun8i-hwspinlock.yaml     | 63 +++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
> new file mode 100644
> index 000000000000..2954ee0b36a7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwlock/sun8i-hwspinlock.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SUN8I hardware spinlock driver for Allwinner sun8i compatible SoCs
> +
> +maintainers:
> +  - Wilken Gottwalt <wilken.gottwalt@posteo.net>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - allwinner,sun8i-hwspinlock

This can be a const instead of an enum, and since it was introduced with
the A33 it should be sun8i-a33-hwspinlock. There's a lot of SoCs in that
family, some without that IP, and we could even see new SoCs in that
family with a different IP at some point.

> +
> +    reg:          # 0x01C18000 (H2+, H3, H5), 0x03004000 (H6), length 0x1000
> +      maxItems: 1

There's no need for those comments

> +
> +    clocks:       # phandle to the reference clock

This should be the description, and it's fairly obvious so you don't
really need that comment.

> +      maxItems: 1
> +
> +    clock-names:  # name of the bus ("ahb")
> +      maxItems: 1

You don't need clock-names if there's a single clock

> +
> +    resets:       # phandle to the reset control
> +      maxItems: 1

Same thing than for the clocks

> +
> +    reset-names:  # name of the bus ("ahb")
> +      maxItems: 1
> +

Ditto

> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +
> +additionalProperties: false
> +
> +examples:
> +
> +  - |
> +    /* H2+ based OrangePi Zero */
> +    hwspinlock: hwspinlock@1C18000 {

Unit-address's are lowercase

> +      compatible = "allwinner,sun8i-hwspinlock";
> +      reg = <0x01c18000 0x1000>;
> +      clocks = <&ccu CLK_BUS_SPINLOCK>;
> +      clock-names = "ahb";
> +      resets = <&ccu RST_BUS_SPINLOCK>;
> +      reset-names = "ahb";
> +    };
> +
> +    /* H6 based OrangePi 3 */
> +    hwspinlock: hwspinlock@3004000 {
> +      compatible = "allwinner,sun8i-hwspinlock";
> +      reg = <0x03004000 0x1000>;
> +      clocks = <&ccu CLK_BUS_SPINLOCK>;
> +      clock-names = "ahb";
> +      resets = <&ccu RST_BUS_SPINLOCK>;
> +      reset-names = "ahb";
> +    };

Different examples should be different items on that list, but both are
essentially the same binding so you can drop one.

Maxime

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

Re: [PATCH v3 2/2] hwspinlock: add sun8i hardware spinlock support

[Maxime Ripard]

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

On Mon, Dec 07, 2020 at 05:05:34PM +0100, Wilken Gottwalt wrote:
> +	io_base = devm_platform_ioremap_resource(pdev, SPINLOCK_BASE_ID);
> +	if (IS_ERR(io_base)) {
> +		err = PTR_ERR(io_base);
> +		dev_err(&pdev->dev, "unable to request MMIO (%d)\n", err);

There's already a message printed by the core if it fails

> +		return err;
> +	}
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	err = devm_add_action_or_reset(&pdev->dev, sun8i_hwspinlock_disable, priv);
> +	if (err) {
> +		dev_err(&pdev->dev, "unable to add disable action\n");
> +		return err;
> +	}

If the next call fails, you're going to free some resources that you
haven't taken in the first place.

> +
> +	priv->ahb_clk = devm_clk_get(&pdev->dev, "ahb");
> +	if (IS_ERR(priv->ahb_clk)) {
> +		err = PTR_ERR(priv->ahb_clk);
> +		dev_err(&pdev->dev, "unable to get AHB clock (%d)\n", err);
> +		return err;
> +	}
> +
> +	priv->reset = devm_reset_control_get_optional(&pdev->dev, "ahb");

Your binding has it mandatory, so you don't really need it to be
optional?

> +	if (IS_ERR(priv->reset)) {
> +		return dev_err_probe(&pdev->dev, PTR_ERR(priv->reset),
> +				     "unable to get reset control\n");
> +	}

You shouldn't have braces on a single line return

> +
> +	err = reset_control_deassert(priv->reset);
> +	if (err) {
> +		dev_err(&pdev->dev, "deassert reset control failure (%d)\n", err);
> +		return err;
> +	}
> +
> +	err = clk_prepare_enable(priv->ahb_clk);
> +	if (err) {
> +		dev_err(&pdev->dev, "unable to prepare AHB clk (%d)\n", err);
> +		return err;
> +	}
> +
> +	/*
> +	 * bit 28 and 29 hold the amount of spinlock banks, but at the same time the datasheet
> +	 * says, bit 30 and 31 are reserved while the values can be 0 to 4, which is not reachable
> +	 * by two bits alone, so the reserved bits are also taken into account
> +	 */
> +	num_banks = readl(io_base + SPINLOCK_SYSSTATUS_REG) >> 28;
> +	switch (num_banks) {
> +	case 1 ... 4:
> +		/*
> +		 * 32, 64, 128 and 256 spinlocks are supported by the hardware implementation,
> +		 * though most of the SoCs support 32 spinlocks only
> +		 */
> +		priv->nlocks = 1 << (4 + num_banks);
> +		break;
> +	default:
> +		dev_err(&pdev->dev, "unsupported hwspinlock setup (%d)\n", num_banks);
> +		return -EINVAL;
> +	}
> +
> +	priv->bank = devm_kzalloc(&pdev->dev, struct_size(priv->bank, lock, priv->nlocks),
> +				  GFP_KERNEL);
> +	if (!priv->bank)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < priv->nlocks; ++i) {
> +		hwlock = &priv->bank->lock[i];
> +		hwlock->priv = io_base + SPINLOCK_LOCK_REGN + sizeof(u32) * i;
> +	}
> +
> +	sun8i_hwspinlock_debugfs_init(priv);
> +	platform_set_drvdata(pdev, priv);
> +
> +	return devm_hwspin_lock_register(&pdev->dev, priv->bank, &sun8i_hwspinlock_ops,
> +					 SPINLOCK_BASE_ID, priv->nlocks);
> +}
> +
> +static const struct of_device_id sun8i_hwspinlock_ids[] = {
> +	{ .compatible = "allwinner,sun8i-hwspinlock", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sun8i_hwspinlock_ids);
> +
> +static struct platform_driver sun8i_hwspinlock_driver = {
> +	.probe	= sun8i_hwspinlock_probe,
> +	.driver	= {
> +		.name		= DRIVER_NAME,
> +		.of_match_table	= sun8i_hwspinlock_ids,
> +	},
> +};
> +
> +static int __init sun8i_hwspinlock_init(void)
> +{
> +	return platform_driver_register(&sun8i_hwspinlock_driver);
> +}
> +module_init(sun8i_hwspinlock_init);
> +
> +static void __exit sun8i_hwspinlock_exit(void)
> +{
> +	platform_driver_unregister(&sun8i_hwspinlock_driver);
> +}
> +module_exit(sun8i_hwspinlock_exit);

This can be replaced by module_platform_driver

Maxime

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

Re: [PATCH v3 1/2] dt-bindings: hwlock: add sun8i_hwspinlock documentation

[Samuel Holland]

On 12/7/20 10:12 AM, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Dec 07, 2020 at 05:05:03PM +0100, Wilken Gottwalt wrote:
>> Adds documentation on how to use the sun8i_hwspinlock driver for sun8i
>> compatible SoCs.
>>
>> Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
>> ---
>>  .../bindings/hwlock/sun8i-hwspinlock.yaml     | 63 +++++++++++++++++++
>>  1 file changed, 63 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
>> new file mode 100644
>> index 000000000000..2954ee0b36a7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
>> @@ -0,0 +1,63 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwlock/sun8i-hwspinlock.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: SUN8I hardware spinlock driver for Allwinner sun8i compatible SoCs
>> +
>> +maintainers:
>> +  - Wilken Gottwalt <wilken.gottwalt@posteo.net>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - allwinner,sun8i-hwspinlock
> 
> This can be a const instead of an enum, and since it was introduced with
> the A33 it should be sun8i-a33-hwspinlock. There's a lot of SoCs in that
> family, some without that IP, and we could even see new SoCs in that
> family with a different IP at some point.

I just looked at the A31 ARISC blob, and it uses the hwspinlock hardware
as well, with the same MMIO address and gate/reset bits as A33-H3. So
the first compatible would actually be sun6i-a31-hwspinlock.

Cheers,
Samuel

>> +
>> +    reg:          # 0x01C18000 (H2+, H3, H5), 0x03004000 (H6), length 0x1000
>> +      maxItems: 1
> 
> There's no need for those comments
> 
>> +
>> +    clocks:       # phandle to the reference clock
> 
> This should be the description, and it's fairly obvious so you don't
> really need that comment.
> 
>> +      maxItems: 1
>> +
>> +    clock-names:  # name of the bus ("ahb")
>> +      maxItems: 1
> 
> You don't need clock-names if there's a single clock
> 
>> +
>> +    resets:       # phandle to the reset control
>> +      maxItems: 1
> 
> Same thing than for the clocks
> 
>> +
>> +    reset-names:  # name of the bus ("ahb")
>> +      maxItems: 1
>> +
> 
> Ditto
> 
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - resets
>> +  - reset-names
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +
>> +  - |
>> +    /* H2+ based OrangePi Zero */
>> +    hwspinlock: hwspinlock@1C18000 {
> 
> Unit-address's are lowercase
> 
>> +      compatible = "allwinner,sun8i-hwspinlock";
>> +      reg = <0x01c18000 0x1000>;
>> +      clocks = <&ccu CLK_BUS_SPINLOCK>;
>> +      clock-names = "ahb";
>> +      resets = <&ccu RST_BUS_SPINLOCK>;
>> +      reset-names = "ahb";
>> +    };
>> +
>> +    /* H6 based OrangePi 3 */
>> +    hwspinlock: hwspinlock@3004000 {
>> +      compatible = "allwinner,sun8i-hwspinlock";
>> +      reg = <0x03004000 0x1000>;
>> +      clocks = <&ccu CLK_BUS_SPINLOCK>;
>> +      clock-names = "ahb";
>> +      resets = <&ccu RST_BUS_SPINLOCK>;
>> +      reset-names = "ahb";
>> +    };
> 
> Different examples should be different items on that list, but both are
> essentially the same binding so you can drop one.
> 
> Maxime
> 

Re: [PATCH v3 1/2] dt-bindings: hwlock: add sun8i_hwspinlock documentation

[Wilken Gottwalt]

On Mon, 7 Dec 2020 21:22:23 -0600
Samuel Holland <samuel@sholland.org> wrote:

> On 12/7/20 10:12 AM, Maxime Ripard wrote:
> > Hi,
> > 
> > On Mon, Dec 07, 2020 at 05:05:03PM +0100, Wilken Gottwalt wrote:
> >> Adds documentation on how to use the sun8i_hwspinlock driver for sun8i
> >> compatible SoCs.
> >>
> >> Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> >> ---
> >>  .../bindings/hwlock/sun8i-hwspinlock.yaml     | 63 +++++++++++++++++++
> >>  1 file changed, 63 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
> >> b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml new file mode 100644
> >> index 000000000000..2954ee0b36a7
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
> >> @@ -0,0 +1,63 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/hwlock/sun8i-hwspinlock.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: SUN8I hardware spinlock driver for Allwinner sun8i compatible SoCs
> >> +
> >> +maintainers:
> >> +  - Wilken Gottwalt <wilken.gottwalt@posteo.net>
> >> +
> >> +properties:
> >> +  compatible:
> >> +    enum:
> >> +      - allwinner,sun8i-hwspinlock
> > 
> > This can be a const instead of an enum, and since it was introduced with
> > the A33 it should be sun8i-a33-hwspinlock. There's a lot of SoCs in that
> > family, some without that IP, and we could even see new SoCs in that
> > family with a different IP at some point.
> 
> I just looked at the A31 ARISC blob, and it uses the hwspinlock hardware
> as well, with the same MMIO address and gate/reset bits as A33-H3. So
> the first compatible would actually be sun6i-a31-hwspinlock.

Hmm, so it would make sense to also change the drivers symbols from sun8i to
sun6i, right? Before I do that, is there maybe a sun4i which also includes
the hwspinlock unit? Just in case :D

greetings,
Wilken

> Cheers,
> Samuel
> 
> >> +
> >> +    reg:          # 0x01C18000 (H2+, H3, H5), 0x03004000 (H6), length 0x1000
> >> +      maxItems: 1
> > 
> > There's no need for those comments
> > 
> >> +
> >> +    clocks:       # phandle to the reference clock
> > 
> > This should be the description, and it's fairly obvious so you don't
> > really need that comment.
> > 
> >> +      maxItems: 1
> >> +
> >> +    clock-names:  # name of the bus ("ahb")
> >> +      maxItems: 1
> > 
> > You don't need clock-names if there's a single clock
> > 
> >> +
> >> +    resets:       # phandle to the reset control
> >> +      maxItems: 1
> > 
> > Same thing than for the clocks
> > 
> >> +
> >> +    reset-names:  # name of the bus ("ahb")
> >> +      maxItems: 1
> >> +
> > 
> > Ditto
> > 
> >> +required:
> >> +  - compatible
> >> +  - reg
> >> +  - clocks
> >> +  - clock-names
> >> +  - resets
> >> +  - reset-names
> >> +
> >> +additionalProperties: false
> >> +
> >> +examples:
> >> +
> >> +  - |
> >> +    /* H2+ based OrangePi Zero */
> >> +    hwspinlock: hwspinlock@1C18000 {
> > 
> > Unit-address's are lowercase
> > 
> >> +      compatible = "allwinner,sun8i-hwspinlock";
> >> +      reg = <0x01c18000 0x1000>;
> >> +      clocks = <&ccu CLK_BUS_SPINLOCK>;
> >> +      clock-names = "ahb";
> >> +      resets = <&ccu RST_BUS_SPINLOCK>;
> >> +      reset-names = "ahb";
> >> +    };
> >> +
> >> +    /* H6 based OrangePi 3 */
> >> +    hwspinlock: hwspinlock@3004000 {
> >> +      compatible = "allwinner,sun8i-hwspinlock";
> >> +      reg = <0x03004000 0x1000>;
> >> +      clocks = <&ccu CLK_BUS_SPINLOCK>;
> >> +      clock-names = "ahb";
> >> +      resets = <&ccu RST_BUS_SPINLOCK>;
> >> +      reset-names = "ahb";
> >> +    };
> > 
> > Different examples should be different items on that list, but both are
> > essentially the same binding so you can drop one.
> > 
> > Maxime
> > 
> 

Re: [PATCH v3 1/2] dt-bindings: hwlock: add sun8i_hwspinlock documentation

[Samuel Holland]

On 12/7/20 11:11 PM, Wilken Gottwalt wrote:
> On Mon, 7 Dec 2020 21:22:23 -0600
> Samuel Holland <samuel@sholland.org> wrote:
> 
>> On 12/7/20 10:12 AM, Maxime Ripard wrote:
>>> Hi,
>>>
>>> On Mon, Dec 07, 2020 at 05:05:03PM +0100, Wilken Gottwalt wrote:
>>>> Adds documentation on how to use the sun8i_hwspinlock driver for sun8i
>>>> compatible SoCs.
>>>>
>>>> Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
>>>> ---
>>>>  .../bindings/hwlock/sun8i-hwspinlock.yaml     | 63 +++++++++++++++++++
>>>>  1 file changed, 63 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
>>>> b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml new file mode 100644
>>>> index 000000000000..2954ee0b36a7
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/hwlock/sun8i-hwspinlock.yaml
>>>> @@ -0,0 +1,63 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/hwlock/sun8i-hwspinlock.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: SUN8I hardware spinlock driver for Allwinner sun8i compatible SoCs
>>>> +
>>>> +maintainers:
>>>> +  - Wilken Gottwalt <wilken.gottwalt@posteo.net>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - allwinner,sun8i-hwspinlock
>>>
>>> This can be a const instead of an enum, and since it was introduced with
>>> the A33 it should be sun8i-a33-hwspinlock. There's a lot of SoCs in that
>>> family, some without that IP, and we could even see new SoCs in that
>>> family with a different IP at some point.
>>
>> I just looked at the A31 ARISC blob, and it uses the hwspinlock hardware
>> as well, with the same MMIO address and gate/reset bits as A33-H3. So
>> the first compatible would actually be sun6i-a31-hwspinlock.
> 
> Hmm, so it would make sense to also change the drivers symbols from sun8i to
> sun6i, right?

Correct.

> Before I do that, is there maybe a sun4i which also includes
> the hwspinlock unit? Just in case :D

There is a sun4i, but there is no evidence it contains the hwspinlock unit.

Cheers,
Samuel

> 
> greetings,
> Wilken
> 
>> Cheers,
>> Samuel
>>
>>>> +
>>>> +    reg:          # 0x01C18000 (H2+, H3, H5), 0x03004000 (H6), length 0x1000
>>>> +      maxItems: 1
>>>
>>> There's no need for those comments
>>>
>>>> +
>>>> +    clocks:       # phandle to the reference clock
>>>
>>> This should be the description, and it's fairly obvious so you don't
>>> really need that comment.
>>>
>>>> +      maxItems: 1
>>>> +
>>>> +    clock-names:  # name of the bus ("ahb")
>>>> +      maxItems: 1
>>>
>>> You don't need clock-names if there's a single clock
>>>
>>>> +
>>>> +    resets:       # phandle to the reset control
>>>> +      maxItems: 1
>>>
>>> Same thing than for the clocks
>>>
>>>> +
>>>> +    reset-names:  # name of the bus ("ahb")
>>>> +      maxItems: 1
>>>> +
>>>
>>> Ditto
>>>
>>>> +required:
>>>> +  - compatible
>>>> +  - reg
>>>> +  - clocks
>>>> +  - clock-names
>>>> +  - resets
>>>> +  - reset-names
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> +
>>>> +  - |
>>>> +    /* H2+ based OrangePi Zero */
>>>> +    hwspinlock: hwspinlock@1C18000 {
>>>
>>> Unit-address's are lowercase
>>>
>>>> +      compatible = "allwinner,sun8i-hwspinlock";
>>>> +      reg = <0x01c18000 0x1000>;
>>>> +      clocks = <&ccu CLK_BUS_SPINLOCK>;
>>>> +      clock-names = "ahb";
>>>> +      resets = <&ccu RST_BUS_SPINLOCK>;
>>>> +      reset-names = "ahb";
>>>> +    };
>>>> +
>>>> +    /* H6 based OrangePi 3 */
>>>> +    hwspinlock: hwspinlock@3004000 {
>>>> +      compatible = "allwinner,sun8i-hwspinlock";
>>>> +      reg = <0x03004000 0x1000>;
>>>> +      clocks = <&ccu CLK_BUS_SPINLOCK>;
>>>> +      clock-names = "ahb";
>>>> +      resets = <&ccu RST_BUS_SPINLOCK>;
>>>> +      reset-names = "ahb";
>>>> +    };
>>>
>>> Different examples should be different items on that list, but both are
>>> essentially the same binding so you can drop one.
>>>
>>> Maxime
>>>
>>
>