* [PATCH v2 0/2] hwspinlock: add sunxi hardware spinlock support @ 2020-12-04 15:33 Wilken Gottwalt 2020-12-04 15:34 ` [PATCH v2 1/2] dt-bindings: hwlock: sunxi: add sunxi_hwspinlock documentation Wilken Gottwalt 2020-12-04 15:35 ` [PATCH v2 2/2] hwspinlock: add sunxi hardware spinlock support Wilken Gottwalt 0 siblings, 2 replies; 7+ messages in thread From: Wilken Gottwalt @ 2020-12-04 15:33 UTC (permalink / raw) To: linux-kernel Cc: Ohad Ben-Cohen, Bjorn Andersson, Baolin Wang, Rob Herring, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec Most of the Allwinner sun8i and sun50i based SoCs contain a spinlock unit which can be used to sync access to devices shared between the ARM cores in the embedded OpenRisc (AR100) core. This spinlock unit is included in the A83T, A64, H2+, H3, H5 and H6 SoCs and according to the datasheets support at least 32 spinlocks. The implementation supports 32, 64, 128 and 256 spinlock setups, but there is no known SoC yet, which supports more the 32. 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_SUNXI symbol. This driver is the first step to enable hwspinlock support in Linux, but also needs support on the OpenRisc core side in the crust firmware. 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. This patch adds: - hwspinlock driver sunxi_hwspinlock - updates makefiles - hwspinlock dt bindings documentation - updates MAINTAINERS Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net> 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: sunxi: add sunxi_hwspinlock documentation hwspinlock: add sunxi hardware spinlock support .../bindings/hwlock/sunxi-hwspinlock.yaml | 64 +++++ MAINTAINERS | 6 + drivers/hwspinlock/Kconfig | 9 + drivers/hwspinlock/Makefile | 1 + drivers/hwspinlock/sunxi_hwspinlock.c | 234 ++++++++++++++++++ 5 files changed, 314 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwlock/sunxi-hwspinlock.yaml create mode 100644 drivers/hwspinlock/sunxi_hwspinlock.c -- 2.29.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] dt-bindings: hwlock: sunxi: add sunxi_hwspinlock documentation 2020-12-04 15:33 [PATCH v2 0/2] hwspinlock: add sunxi hardware spinlock support Wilken Gottwalt @ 2020-12-04 15:34 ` Wilken Gottwalt 2020-12-04 15:35 ` [PATCH v2 2/2] hwspinlock: add sunxi hardware spinlock support Wilken Gottwalt 1 sibling, 0 replies; 7+ messages in thread From: Wilken Gottwalt @ 2020-12-04 15:34 UTC (permalink / raw) To: linux-kernel Cc: Ohad Ben-Cohen, Bjorn Andersson, Baolin Wang, Rob Herring, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec Adds documentation on how to use the sunxi_hwspinlock driver for the sun8i and sun50i based SoCs. Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net> --- Changes in v2: - fixed memory ranges --- .../bindings/hwlock/sunxi-hwspinlock.yaml | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwlock/sunxi-hwspinlock.yaml diff --git a/Documentation/devicetree/bindings/hwlock/sunxi-hwspinlock.yaml b/Documentation/devicetree/bindings/hwlock/sunxi-hwspinlock.yaml new file mode 100644 index 000000000000..32d7e897ead6 --- /dev/null +++ b/Documentation/devicetree/bindings/hwlock/sunxi-hwspinlock.yaml @@ -0,0 +1,64 @@ +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwlock/sunxi-hwspinlock.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: SUNXI hardware spinlock for Allwinner sun8i and sun50i based SoCs + +maintainers: + - Wilken Gottwalt <wilken.gottwalt@posteo.net> + +properties: + compatible: + enum: + - allwinner,sun8i-hwspinlock + - allwinner,sun50i-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,sun50i-hwspinlock"; + reg = <0x03004000 0x1000>; + clocks = <&ccu CLK_BUS_SPINLOCK>; + clock-names = "ahb"; + resets = <&ccu RST_BUS_SPINLOCK>; + reset-names = "ahb"; + }; -- 2.29.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] hwspinlock: add sunxi hardware spinlock support 2020-12-04 15:33 [PATCH v2 0/2] hwspinlock: add sunxi hardware spinlock support Wilken Gottwalt 2020-12-04 15:34 ` [PATCH v2 1/2] dt-bindings: hwlock: sunxi: add sunxi_hwspinlock documentation Wilken Gottwalt @ 2020-12-04 15:35 ` Wilken Gottwalt 2020-12-04 16:21 ` Maxime Ripard 1 sibling, 1 reply; 7+ messages in thread From: Wilken Gottwalt @ 2020-12-04 15:35 UTC (permalink / raw) To: linux-kernel Cc: Ohad Ben-Cohen, Bjorn Andersson, Baolin Wang, Rob Herring, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec Adds the sunxi_hwspinlock driver for the hardware spinlock unit found in most of the sun8i and sun50i based SoCs. This unit provides at least 32 spinlocks in hardware. The implementation supports 32, 64, 128 or 256 32bit registers, where one 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 currently 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 the 32 first locks only. For this reason the lock read/write approach is used in this driver which can cover all 256 locks in the biggest setup. Being able to use the status register to get knowledge about the locks makes it suitable to extended testing. It also can be used to bypass the Linux hwspinlock ABI completely and is not able to set locks. This is used in one of the tests to show that the driver works correctly. 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 crust. In this case a H5 based device works very well. In this test a Friendlyarm NanoPi NEO2 was used, which is fully supported by u-boot (also the fork) 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, which uses the timeout functions of the crust firmware. It loops through the first 32 spinlocks and takes/releases a lock one after another using a timeout which can be set in the debug menu. Test 1: This test was done with a mainline u-boot and a crust enabled u-boot. For this a simple second kernel module was used, found here https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test_module. If run with 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. Done for all 32 locks several times. [ 122.518435] [init]--- SUNXI HWSPINLOCK DRIVER TEST --- [ 122.523810] [run ]--- testing locks 0 to 31 --- [ 122.528370] [test] testing lock 0 [ 122.531686] [test]+++ attempt #0 succeded [ 122.535709] [test]+++ attempt #1 succeded [ 122.539729] [test]+++ attempt #2 succeded [ 122.543752] [test] testing lock 1 [ 122.547072] [test]+++ attempt #0 succeded [ 122.551093] [test]+++ attempt #1 succeded [ 122.555113] [test]+++ attempt #2 succeded [ 122.559129] [test] testing lock 2 ... [ 123.004812] [test] testing lock 31 [ 123.008208] [test]+++ attempt #0 succeded [ 123.012228] [test]+++ attempt #1 succeded [ 123.016250] [test]+++ attempt #2 succeded Same test run with the hwspinlock test loop enabled in crust. Here the test module hits two locks exactly in the moment crust switches from lock 7 to 8. The test module code path is slower because of more code that needs to be executed which makes this rare scenario possible. [ 122.620656] [test] testing lock 6 [ 122.623966] [test]+++ attempt #0 succeded [ 122.627988] [test]+++ attempt #1 succeded [ 122.632008] [test]+++ attempt #2 succeded [ 122.636044] [test] testing lock 7 [ 122.639364] [test] taking lock attempt #0 failed (-16) [ 122.644507] [run ]--- testing specific lock 7 failed (-14) --- [ 122.650345] [test] testing lock 8 [ 122.653671] [test] taking lock attempt #0 failed (-16) [ 122.658818] [run ]--- testing specific lock 8 failed (-14) --- [ 122.664658] [test] testing lock 9 [ 122.667971] [test]+++ attempt #0 succeded [ 122.671989] [test]+++ attempt #1 succeded Test 2: This is a more complex test that uses the status register to bypass the Linux hwspinlock ABI. For this to work a slightly modified driver is used and can be found here https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/ modified_sunxi_hwspinlock 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 module which can be found here https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test2_module It is also necessary to change the device tree entries to get both kernel modules working in parallel. hwspinlock-mod@1c18000 { compatible = "allwinner,sun50i-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,sun50i-hwspinlock-stat"; reg = <0x01c18010 0x4>; status = "okay"; }; The extended test kernel module supports 4 different modes of the test, 2 of them are sufficient to show the spinlock mechanism working. Mode 1: This one reads and prints the status register continuously. The crust firmware and the test are set to a hwlock timeout of 1 second. The test kernel module code runs a bit slower because of more code executed and you can see how one lock is missed between entry 2 and 3. > modprobe sunxi_hwspinlock_test2 mode=1 loops=10 [ 187.167074] [init]--- SUNXI HWSPINLOCK DRIVER TEST --- [ 187.172636] [sreg] 00000000_00010000_00000000_00000000 [ 188.196287] [sreg] 00000000_00001000_00000000_00000000 [ 189.220285] [sreg] 00000000_00000010_00000000_00000000 [ 190.244286] [sreg] 00000000_00000001_00000000_00000000 [ 191.268285] [sreg] 00000000_00000000_10000000_00000000 [ 192.292294] [sreg] 00000000_00000000_01000000_00000000 [ 193.316302] [sreg] 00000000_00000000_00100000_00000000 [ 194.340285] [sreg] 00000000_00000000_00010000_00000000 [ 195.364285] [sreg] 00000000_00000000_00001000_00000000 [ 196.388284] [sreg] 00000000_00000000_00000100_00000000 Mode 3: This mode combines the Linux hwspinlock ABI approach from test 1 and the status register access. The "after" reads show the locks taken by the Linux driver and the crust firmware. [ 439.138476] [test] testing lock 13 [ 439.141894] [sreg] before take 00000000_00000100_00000000_00000000 [ 439.148083] [sreg] after take 00000000_00000110_00000000_00000000 [ 439.154189] [sreg] after recursive take 00000000_00000110_00000000_00000000 [ 439.161162] [sreg] after untake 00000000_00000010_00000000_00000000 [ 439.167435] [test]+++ attempt #0 succeded [ 439.171458] [sreg] before take 00000000_00000010_00000000_00000000 [ 439.177649] [sreg] after take 00000000_00000110_00000000_00000000 [ 439.183751] [sreg] after recursive take 00000000_00000110_00000000_00000000 [ 439.190725] [sreg] after untake 00000000_00000010_00000000_00000000 [ 439.196992] [test]+++ attempt #1 succeded [ 439.201018] [sreg] before take 00000000_00000010_00000000_00000000 [ 439.207212] [sreg] after take 00000000_00000110_00000000_00000000 [ 439.213322] [sreg] after recursive take 00000000_00000110_00000000_00000000 [ 439.220290] [sreg] after untake 00000000_00000010_00000000_00000000 [ 439.226559] [test]+++ attempt #2 succeded [ 439.230576] [test] testing lock 14 [ 439.233996] [sreg] before take 00000000_00000010_00000000_00000000 [ 439.240177] [test] taking lock attempt #0 failed (-16) [ 439.245322] [run ]--- testing specific lock 14 failed (-14) --- [ 439.251252] [test] testing lock 15 [ 439.254677] [sreg] before take 00000000_00000010_00000000_00000000 [ 439.260865] [sreg] after take 00000000_00000011_00000000_00000000 [ 439.266975] [sreg] after recursive take 00000000_00000011_00000000_00000000 [ 439.273943] [sreg] after untake 00000000_00000010_00000000_00000000 [ 439.280212] [test]+++ attempt #0 succeded [ 439.284235] [sreg] before take 00000000_00000010_00000000_00000000 [ 439.290428] [sreg] after take 00000000_00000011_00000000_00000000 [ 439.296534] [sreg] after recursive take 00000000_00000011_00000000_00000000 [ 439.303502] [sreg] after untake 00000000_00000010_00000000_00000000 [ 439.309774] [test]+++ attempt #1 succeded Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net> --- 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/sunxi_hwspinlock.c | 234 ++++++++++++++++++++++++++ 4 files changed, 250 insertions(+) create mode 100644 drivers/hwspinlock/sunxi_hwspinlock.c diff --git a/MAINTAINERS b/MAINTAINERS index ebe4829cdd4d..29a96a043cee 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/sunxi-hwspinlock.yaml +F: drivers/hwspinlock/sunxi_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..a568a4b5010c 100644 --- a/drivers/hwspinlock/Kconfig +++ b/drivers/hwspinlock/Kconfig @@ -55,6 +55,15 @@ config HWSPINLOCK_STM32 If unsure, say N. +config HWSPINLOCK_SUNXI + tristate "SUNXI Hardware Spinlock device" + depends on ARCH_SUNXI || COMPILE_TEST + help + Say y here to support the Allwinner hardware mutex device available + in most of the sun8i and sun50i 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..bf46bee95226 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_SUNXI) += sunxi_hwspinlock.o obj-$(CONFIG_HSEM_U8500) += u8500_hsem.o diff --git a/drivers/hwspinlock/sunxi_hwspinlock.c b/drivers/hwspinlock/sunxi_hwspinlock.c new file mode 100644 index 000000000000..91adb2743aba --- /dev/null +++ b/drivers/hwspinlock/sunxi_hwspinlock.c @@ -0,0 +1,234 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * sunxi_hwspinlock.c - hardware spinlock driver for 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 "sunxi_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 sunxi_hwspinlock_data { + struct hwspinlock_device *bank; + struct reset_control *reset; + struct clk *ahb_clock; + struct dentry *debugfs; + int nlocks; +}; + +#ifdef CONFIG_DEBUG_FS + +static int hwlocks_supported_show(struct seq_file *seqf, void *unused) +{ + struct sunxi_hwspinlock_data *priv = seqf->private; + + seq_printf(seqf, "%d\n", priv->nlocks); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(hwlocks_supported); + +static void sunxi_hwspinlock_debugfs_init(struct sunxi_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 sunxi_hwspinlock_debugfs_init(struct sunxi_hwspinlock_data *priv) +{ +} + +#endif + +static int sunxi_hwspinlock_trylock(struct hwspinlock *lock) +{ + void __iomem *lock_addr = lock->priv; + + return (readl(lock_addr) == SPINLOCK_NOTTAKEN); +} + +static void sunxi_hwspinlock_unlock(struct hwspinlock *lock) +{ + void __iomem *lock_addr = lock->priv; + + writel(SPINLOCK_NOTTAKEN, lock_addr); +} + +static const struct hwspinlock_ops sunxi_hwspinlock_ops = { + .trylock = sunxi_hwspinlock_trylock, + .unlock = sunxi_hwspinlock_unlock, +}; + +static int sunxi_hwspinlock_probe(struct platform_device *pdev) +{ + struct sunxi_hwspinlock_data *priv; + struct hwspinlock *hwlock; + void __iomem *io_base; + int num_banks, 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; + + priv->ahb_clock = devm_clk_get(&pdev->dev, "ahb"); + if (IS_ERR(priv->ahb_clock)) { + err = PTR_ERR(priv->ahb_clock); + 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_clock); + if (err) { + dev_err(&pdev->dev, "unable to prepare AHB clock (%d)\n", err); + goto reset_fail; + } + + /* + * 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 + * + * order is important here, getting the amount of locks is only possible after setting up + * clocks and resets + */ + 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 << (5 + i); + 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) { + err = -ENOMEM; + goto reset_fail; + } + + for (i = 0; i < priv->nlocks; ++i) { + hwlock = &priv->bank->lock[i]; + hwlock->priv = io_base + SPINLOCK_LOCK_REGN + sizeof(u32) * i; + } + + err = hwspin_lock_register(priv->bank, &pdev->dev, &sunxi_hwspinlock_ops, SPINLOCK_BASE_ID, + priv->nlocks); + if (err) { + dev_err(&pdev->dev, "unable to register hwspinlocks (%d)\n", err); + goto fail; + } + + sunxi_hwspinlock_debugfs_init(priv); + platform_set_drvdata(pdev, priv); + + dev_dbg(&pdev->dev, "SUNXI hardware spinlock driver enabled (%d locks)\n", priv->nlocks); + + return 0; + +fail: + clk_disable_unprepare(priv->ahb_clock); + +reset_fail: + if (priv->reset) + reset_control_assert(priv->reset); + + return err; +} + +static int sunxi_hwspinlock_remove(struct platform_device *pdev) +{ + struct sunxi_hwspinlock_data *priv = platform_get_drvdata(pdev); + int err; + + debugfs_remove_recursive(priv->debugfs); + + err = hwspin_lock_unregister(priv->bank); + if (err) { + dev_err(&pdev->dev, "unregister device failed (%d)\n", err); + return err; + } + + if (priv->reset) + reset_control_assert(priv->reset); + + clk_disable_unprepare(priv->ahb_clock); + + return 0; +} + +static const struct of_device_id sunxi_hwspinlock_ids[] = { + { .compatible = "allwinner,sun8i-hwspinlock", }, + { .compatible = "allwinner,sun50i-hwspinlock", }, + {}, +}; +MODULE_DEVICE_TABLE(of, sunxi_hwspinlock_ids); + +static struct platform_driver sunxi_hwspinlock_driver = { + .probe = sunxi_hwspinlock_probe, + .remove = sunxi_hwspinlock_remove, + .driver = { + .name = DRIVER_NAME, + .of_match_table = sunxi_hwspinlock_ids, + }, +}; + +static int __init sunxi_hwspinlock_init(void) +{ + return platform_driver_register(&sunxi_hwspinlock_driver); +} +postcore_initcall(sunxi_hwspinlock_init); + +static void __exit sunxi_hwspinlock_exit(void) +{ + platform_driver_unregister(&sunxi_hwspinlock_driver); +} +module_exit(sunxi_hwspinlock_exit); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("SUNXI hardware spinlock driver"); +MODULE_AUTHOR("Wilken Gottwalt <wilken.gottwalt@posteo.net>"); -- 2.29.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] hwspinlock: add sunxi hardware spinlock support 2020-12-04 15:35 ` [PATCH v2 2/2] hwspinlock: add sunxi hardware spinlock support Wilken Gottwalt @ 2020-12-04 16:21 ` Maxime Ripard 2020-12-04 16:56 ` Wilken Gottwalt 2020-12-04 17:07 ` Wilken Gottwalt 0 siblings, 2 replies; 7+ messages in thread From: Maxime Ripard @ 2020-12-04 16:21 UTC (permalink / raw) To: Wilken Gottwalt Cc: linux-kernel, Ohad Ben-Cohen, Bjorn Andersson, Baolin Wang, Rob Herring, Chen-Yu Tsai, Jernej Skrabec [-- Attachment #1: Type: text/plain, Size: 10122 bytes --] On Fri, Dec 04, 2020 at 04:35:12PM +0100, Wilken Gottwalt wrote: > Adds the sunxi_hwspinlock driver for the hardware spinlock unit found in > most of the sun8i and sun50i based SoCs. > > This unit provides at least 32 spinlocks in hardware. The implementation > supports 32, 64, 128 or 256 32bit registers, where one 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 > currently 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 the 32 first locks only. For > this reason the lock read/write approach is used in this driver which > can cover all 256 locks in the biggest setup. > > Being able to use the status register to get knowledge about the locks > makes it suitable to extended testing. It also can be used to bypass > the Linux hwspinlock ABI completely and is not able to set locks. This > is used in one of the tests to show that the driver works correctly. > > 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 crust. In this case a H5 based device works very well. In > this test a Friendlyarm NanoPi NEO2 was used, which is fully supported > by u-boot (also the fork) 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, which uses the timeout functions of the crust > firmware. It loops through the first 32 spinlocks and takes/releases a > lock one after another using a timeout which can be set in the debug > menu. > > Test 1: > This test was done with a mainline u-boot and a crust enabled u-boot. > For this a simple second kernel module was used, found here > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test_module. > > If run with 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. Done for all 32 locks several times. > > [ 122.518435] [init]--- SUNXI HWSPINLOCK DRIVER TEST --- > [ 122.523810] [run ]--- testing locks 0 to 31 --- > [ 122.528370] [test] testing lock 0 > [ 122.531686] [test]+++ attempt #0 succeded > [ 122.535709] [test]+++ attempt #1 succeded > [ 122.539729] [test]+++ attempt #2 succeded > [ 122.543752] [test] testing lock 1 > [ 122.547072] [test]+++ attempt #0 succeded > [ 122.551093] [test]+++ attempt #1 succeded > [ 122.555113] [test]+++ attempt #2 succeded > [ 122.559129] [test] testing lock 2 > ... > [ 123.004812] [test] testing lock 31 > [ 123.008208] [test]+++ attempt #0 succeded > [ 123.012228] [test]+++ attempt #1 succeded > [ 123.016250] [test]+++ attempt #2 succeded > > Same test run with the hwspinlock test loop enabled in crust. Here the > test module hits two locks exactly in the moment crust switches from > lock 7 to 8. The test module code path is slower because of more code > that needs to be executed which makes this rare scenario possible. > > [ 122.620656] [test] testing lock 6 > [ 122.623966] [test]+++ attempt #0 succeded > [ 122.627988] [test]+++ attempt #1 succeded > [ 122.632008] [test]+++ attempt #2 succeded > [ 122.636044] [test] testing lock 7 > [ 122.639364] [test] taking lock attempt #0 failed (-16) > [ 122.644507] [run ]--- testing specific lock 7 failed (-14) --- > [ 122.650345] [test] testing lock 8 > [ 122.653671] [test] taking lock attempt #0 failed (-16) > [ 122.658818] [run ]--- testing specific lock 8 failed (-14) --- > [ 122.664658] [test] testing lock 9 > [ 122.667971] [test]+++ attempt #0 succeded > [ 122.671989] [test]+++ attempt #1 succeded > > Test 2: > This is a more complex test that uses the status register to bypass the > Linux hwspinlock ABI. For this to work a slightly modified driver is > used and can be found here > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/ > modified_sunxi_hwspinlock > 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 > module which can be found here > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test2_module > It is also necessary to change the device tree entries to get both > kernel modules working in parallel. > > hwspinlock-mod@1c18000 { > compatible = "allwinner,sun50i-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,sun50i-hwspinlock-stat"; > reg = <0x01c18010 0x4>; > status = "okay"; > }; > > The extended test kernel module supports 4 different modes of the test, > 2 of them are sufficient to show the spinlock mechanism working. > > Mode 1: > This one reads and prints the status register continuously. The crust > firmware and the test are set to a hwlock timeout of 1 second. The test > kernel module code runs a bit slower because of more code executed and > you can see how one lock is missed between entry 2 and 3. > > > modprobe sunxi_hwspinlock_test2 mode=1 loops=10 > [ 187.167074] [init]--- SUNXI HWSPINLOCK DRIVER TEST --- > [ 187.172636] [sreg] 00000000_00010000_00000000_00000000 > [ 188.196287] [sreg] 00000000_00001000_00000000_00000000 > [ 189.220285] [sreg] 00000000_00000010_00000000_00000000 > [ 190.244286] [sreg] 00000000_00000001_00000000_00000000 > [ 191.268285] [sreg] 00000000_00000000_10000000_00000000 > [ 192.292294] [sreg] 00000000_00000000_01000000_00000000 > [ 193.316302] [sreg] 00000000_00000000_00100000_00000000 > [ 194.340285] [sreg] 00000000_00000000_00010000_00000000 > [ 195.364285] [sreg] 00000000_00000000_00001000_00000000 > [ 196.388284] [sreg] 00000000_00000000_00000100_00000000 > > Mode 3: > This mode combines the Linux hwspinlock ABI approach from test 1 and the > status register access. The "after" reads show the locks taken by the > Linux driver and the crust firmware. > > [ 439.138476] [test] testing lock 13 > [ 439.141894] [sreg] before take 00000000_00000100_00000000_00000000 > [ 439.148083] [sreg] after take 00000000_00000110_00000000_00000000 > [ 439.154189] [sreg] after recursive take 00000000_00000110_00000000_00000000 > [ 439.161162] [sreg] after untake 00000000_00000010_00000000_00000000 > [ 439.167435] [test]+++ attempt #0 succeded > [ 439.171458] [sreg] before take 00000000_00000010_00000000_00000000 > [ 439.177649] [sreg] after take 00000000_00000110_00000000_00000000 > [ 439.183751] [sreg] after recursive take 00000000_00000110_00000000_00000000 > [ 439.190725] [sreg] after untake 00000000_00000010_00000000_00000000 > [ 439.196992] [test]+++ attempt #1 succeded > [ 439.201018] [sreg] before take 00000000_00000010_00000000_00000000 > [ 439.207212] [sreg] after take 00000000_00000110_00000000_00000000 > [ 439.213322] [sreg] after recursive take 00000000_00000110_00000000_00000000 > [ 439.220290] [sreg] after untake 00000000_00000010_00000000_00000000 > [ 439.226559] [test]+++ attempt #2 succeded > [ 439.230576] [test] testing lock 14 > [ 439.233996] [sreg] before take 00000000_00000010_00000000_00000000 > [ 439.240177] [test] taking lock attempt #0 failed (-16) > [ 439.245322] [run ]--- testing specific lock 14 failed (-14) --- > [ 439.251252] [test] testing lock 15 > [ 439.254677] [sreg] before take 00000000_00000010_00000000_00000000 > [ 439.260865] [sreg] after take 00000000_00000011_00000000_00000000 > [ 439.266975] [sreg] after recursive take 00000000_00000011_00000000_00000000 > [ 439.273943] [sreg] after untake 00000000_00000010_00000000_00000000 > [ 439.280212] [test]+++ attempt #0 succeded > [ 439.284235] [sreg] before take 00000000_00000010_00000000_00000000 > [ 439.290428] [sreg] after take 00000000_00000011_00000000_00000000 > [ 439.296534] [sreg] after recursive take 00000000_00000011_00000000_00000000 > [ 439.303502] [sreg] after untake 00000000_00000010_00000000_00000000 > [ 439.309774] [test]+++ attempt #1 succeded Most of this should be in the cover letter (the details on how to make the tests mostly). However, I mentioned in your first version that some comments on the previous drivers posted still applied to you, and you missed those comments apparently. See here for more details: https://patchwork.kernel.org/project/linux-arm-kernel/patch/20200210170143.20007-2-nborisov@suse.com/#23148161 Most importantly, the driver name and compatible need to be changed. Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] hwspinlock: add sunxi hardware spinlock support 2020-12-04 16:21 ` Maxime Ripard @ 2020-12-04 16:56 ` Wilken Gottwalt 2020-12-04 17:07 ` Wilken Gottwalt 1 sibling, 0 replies; 7+ messages in thread From: Wilken Gottwalt @ 2020-12-04 16:56 UTC (permalink / raw) To: Maxime Ripard Cc: linux-kernel, Ohad Ben-Cohen, Bjorn Andersson, Baolin Wang, Rob Herring, Chen-Yu Tsai, Jernej Skrabec On Fri, 4 Dec 2020 17:21:17 +0100 Maxime Ripard <maxime@cerno.tech> wrote: > On Fri, Dec 04, 2020 at 04:35:12PM +0100, Wilken Gottwalt wrote: > > Adds the sunxi_hwspinlock driver for the hardware spinlock unit found in > > most of the sun8i and sun50i based SoCs. > > > > This unit provides at least 32 spinlocks in hardware. The implementation > > supports 32, 64, 128 or 256 32bit registers, where one 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 > > currently 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 the 32 first locks only. For > > this reason the lock read/write approach is used in this driver which > > can cover all 256 locks in the biggest setup. > > > > Being able to use the status register to get knowledge about the locks > > makes it suitable to extended testing. It also can be used to bypass > > the Linux hwspinlock ABI completely and is not able to set locks. This > > is used in one of the tests to show that the driver works correctly. > > > > 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 crust. In this case a H5 based device works very well. In > > this test a Friendlyarm NanoPi NEO2 was used, which is fully supported > > by u-boot (also the fork) 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, which uses the timeout functions of the crust > > firmware. It loops through the first 32 spinlocks and takes/releases a > > lock one after another using a timeout which can be set in the debug > > menu. > > > > Test 1: > > This test was done with a mainline u-boot and a crust enabled u-boot. > > For this a simple second kernel module was used, found here > > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test_module. > > > > If run with 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. Done for all 32 locks several times. > > > > [ 122.518435] [init]--- SUNXI HWSPINLOCK DRIVER TEST --- > > [ 122.523810] [run ]--- testing locks 0 to 31 --- > > [ 122.528370] [test] testing lock 0 > > [ 122.531686] [test]+++ attempt #0 succeded > > [ 122.535709] [test]+++ attempt #1 succeded > > [ 122.539729] [test]+++ attempt #2 succeded > > [ 122.543752] [test] testing lock 1 > > [ 122.547072] [test]+++ attempt #0 succeded > > [ 122.551093] [test]+++ attempt #1 succeded > > [ 122.555113] [test]+++ attempt #2 succeded > > [ 122.559129] [test] testing lock 2 > > ... > > [ 123.004812] [test] testing lock 31 > > [ 123.008208] [test]+++ attempt #0 succeded > > [ 123.012228] [test]+++ attempt #1 succeded > > [ 123.016250] [test]+++ attempt #2 succeded > > > > Same test run with the hwspinlock test loop enabled in crust. Here the > > test module hits two locks exactly in the moment crust switches from > > lock 7 to 8. The test module code path is slower because of more code > > that needs to be executed which makes this rare scenario possible. > > > > [ 122.620656] [test] testing lock 6 > > [ 122.623966] [test]+++ attempt #0 succeded > > [ 122.627988] [test]+++ attempt #1 succeded > > [ 122.632008] [test]+++ attempt #2 succeded > > [ 122.636044] [test] testing lock 7 > > [ 122.639364] [test] taking lock attempt #0 failed (-16) > > [ 122.644507] [run ]--- testing specific lock 7 failed (-14) --- > > [ 122.650345] [test] testing lock 8 > > [ 122.653671] [test] taking lock attempt #0 failed (-16) > > [ 122.658818] [run ]--- testing specific lock 8 failed (-14) --- > > [ 122.664658] [test] testing lock 9 > > [ 122.667971] [test]+++ attempt #0 succeded > > [ 122.671989] [test]+++ attempt #1 succeded > > > > Test 2: > > This is a more complex test that uses the status register to bypass the > > Linux hwspinlock ABI. For this to work a slightly modified driver is > > used and can be found here > > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/ > > modified_sunxi_hwspinlock > > 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 > > module which can be found here > > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test2_module > > It is also necessary to change the device tree entries to get both > > kernel modules working in parallel. > > > > hwspinlock-mod@1c18000 { > > compatible = "allwinner,sun50i-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,sun50i-hwspinlock-stat"; > > reg = <0x01c18010 0x4>; > > status = "okay"; > > }; > > > > The extended test kernel module supports 4 different modes of the test, > > 2 of them are sufficient to show the spinlock mechanism working. > > > > Mode 1: > > This one reads and prints the status register continuously. The crust > > firmware and the test are set to a hwlock timeout of 1 second. The test > > kernel module code runs a bit slower because of more code executed and > > you can see how one lock is missed between entry 2 and 3. > > > > > modprobe sunxi_hwspinlock_test2 mode=1 loops=10 > > [ 187.167074] [init]--- SUNXI HWSPINLOCK DRIVER TEST --- > > [ 187.172636] [sreg] 00000000_00010000_00000000_00000000 > > [ 188.196287] [sreg] 00000000_00001000_00000000_00000000 > > [ 189.220285] [sreg] 00000000_00000010_00000000_00000000 > > [ 190.244286] [sreg] 00000000_00000001_00000000_00000000 > > [ 191.268285] [sreg] 00000000_00000000_10000000_00000000 > > [ 192.292294] [sreg] 00000000_00000000_01000000_00000000 > > [ 193.316302] [sreg] 00000000_00000000_00100000_00000000 > > [ 194.340285] [sreg] 00000000_00000000_00010000_00000000 > > [ 195.364285] [sreg] 00000000_00000000_00001000_00000000 > > [ 196.388284] [sreg] 00000000_00000000_00000100_00000000 > > > > Mode 3: > > This mode combines the Linux hwspinlock ABI approach from test 1 and the > > status register access. The "after" reads show the locks taken by the > > Linux driver and the crust firmware. > > > > [ 439.138476] [test] testing lock 13 > > [ 439.141894] [sreg] before take 00000000_00000100_00000000_00000000 > > [ 439.148083] [sreg] after take 00000000_00000110_00000000_00000000 > > [ 439.154189] [sreg] after recursive take 00000000_00000110_00000000_00000000 > > [ 439.161162] [sreg] after untake 00000000_00000010_00000000_00000000 > > [ 439.167435] [test]+++ attempt #0 succeded > > [ 439.171458] [sreg] before take 00000000_00000010_00000000_00000000 > > [ 439.177649] [sreg] after take 00000000_00000110_00000000_00000000 > > [ 439.183751] [sreg] after recursive take 00000000_00000110_00000000_00000000 > > [ 439.190725] [sreg] after untake 00000000_00000010_00000000_00000000 > > [ 439.196992] [test]+++ attempt #1 succeded > > [ 439.201018] [sreg] before take 00000000_00000010_00000000_00000000 > > [ 439.207212] [sreg] after take 00000000_00000110_00000000_00000000 > > [ 439.213322] [sreg] after recursive take 00000000_00000110_00000000_00000000 > > [ 439.220290] [sreg] after untake 00000000_00000010_00000000_00000000 > > [ 439.226559] [test]+++ attempt #2 succeded > > [ 439.230576] [test] testing lock 14 > > [ 439.233996] [sreg] before take 00000000_00000010_00000000_00000000 > > [ 439.240177] [test] taking lock attempt #0 failed (-16) > > [ 439.245322] [run ]--- testing specific lock 14 failed (-14) --- > > [ 439.251252] [test] testing lock 15 > > [ 439.254677] [sreg] before take 00000000_00000010_00000000_00000000 > > [ 439.260865] [sreg] after take 00000000_00000011_00000000_00000000 > > [ 439.266975] [sreg] after recursive take 00000000_00000011_00000000_00000000 > > [ 439.273943] [sreg] after untake 00000000_00000010_00000000_00000000 > > [ 439.280212] [test]+++ attempt #0 succeded > > [ 439.284235] [sreg] before take 00000000_00000010_00000000_00000000 > > [ 439.290428] [sreg] after take 00000000_00000011_00000000_00000000 > > [ 439.296534] [sreg] after recursive take 00000000_00000011_00000000_00000000 > > [ 439.303502] [sreg] after untake 00000000_00000010_00000000_00000000 > > [ 439.309774] [test]+++ attempt #1 succeded > > Most of this should be in the cover letter (the details on how to make the tests mostly). > > However, I mentioned in your first version that some comments on the > previous drivers posted still applied to you, and you missed those > comments apparently. See here for more details: > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20200210170143.20007-2-nborisov@suse.com/#23148161 > > Most importantly, the driver name and compatible need to be changed. Ah yes, I really missed them, sorry for that. Getting the tests right and working got all my attention. So does it mean the tests are sufficient now? greetings, Wilken > Maxime ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] hwspinlock: add sunxi hardware spinlock support 2020-12-04 16:21 ` Maxime Ripard 2020-12-04 16:56 ` Wilken Gottwalt @ 2020-12-04 17:07 ` Wilken Gottwalt 2020-12-07 9:26 ` Maxime Ripard 1 sibling, 1 reply; 7+ messages in thread From: Wilken Gottwalt @ 2020-12-04 17:07 UTC (permalink / raw) To: Maxime Ripard Cc: linux-kernel, Ohad Ben-Cohen, Bjorn Andersson, Baolin Wang, Rob Herring, Chen-Yu Tsai, Jernej Skrabec On Fri, 4 Dec 2020 17:21:17 +0100 Maxime Ripard <maxime@cerno.tech> wrote: > On Fri, Dec 04, 2020 at 04:35:12PM +0100, Wilken Gottwalt wrote: > > Adds the sunxi_hwspinlock driver for the hardware spinlock unit found in > > most of the sun8i and sun50i based SoCs. > > > > This unit provides at least 32 spinlocks in hardware. The implementation > > supports 32, 64, 128 or 256 32bit registers, where one 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 > > currently 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 the 32 first locks only. For > > this reason the lock read/write approach is used in this driver which > > can cover all 256 locks in the biggest setup. > > > > Being able to use the status register to get knowledge about the locks > > makes it suitable to extended testing. It also can be used to bypass > > the Linux hwspinlock ABI completely and is not able to set locks. This > > is used in one of the tests to show that the driver works correctly. > > > > 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 crust. In this case a H5 based device works very well. In > > this test a Friendlyarm NanoPi NEO2 was used, which is fully supported > > by u-boot (also the fork) 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, which uses the timeout functions of the crust > > firmware. It loops through the first 32 spinlocks and takes/releases a > > lock one after another using a timeout which can be set in the debug > > menu. > > > > Test 1: > > This test was done with a mainline u-boot and a crust enabled u-boot. > > For this a simple second kernel module was used, found here > > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test_module. > > > > If run with 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. Done for all 32 locks several times. > > > > [ 122.518435] [init]--- SUNXI HWSPINLOCK DRIVER TEST --- > > [ 122.523810] [run ]--- testing locks 0 to 31 --- > > [ 122.528370] [test] testing lock 0 > > [ 122.531686] [test]+++ attempt #0 succeded > > [ 122.535709] [test]+++ attempt #1 succeded > > [ 122.539729] [test]+++ attempt #2 succeded > > [ 122.543752] [test] testing lock 1 > > [ 122.547072] [test]+++ attempt #0 succeded > > [ 122.551093] [test]+++ attempt #1 succeded > > [ 122.555113] [test]+++ attempt #2 succeded > > [ 122.559129] [test] testing lock 2 > > ... > > [ 123.004812] [test] testing lock 31 > > [ 123.008208] [test]+++ attempt #0 succeded > > [ 123.012228] [test]+++ attempt #1 succeded > > [ 123.016250] [test]+++ attempt #2 succeded > > > > Same test run with the hwspinlock test loop enabled in crust. Here the > > test module hits two locks exactly in the moment crust switches from > > lock 7 to 8. The test module code path is slower because of more code > > that needs to be executed which makes this rare scenario possible. > > > > [ 122.620656] [test] testing lock 6 > > [ 122.623966] [test]+++ attempt #0 succeded > > [ 122.627988] [test]+++ attempt #1 succeded > > [ 122.632008] [test]+++ attempt #2 succeded > > [ 122.636044] [test] testing lock 7 > > [ 122.639364] [test] taking lock attempt #0 failed (-16) > > [ 122.644507] [run ]--- testing specific lock 7 failed (-14) --- > > [ 122.650345] [test] testing lock 8 > > [ 122.653671] [test] taking lock attempt #0 failed (-16) > > [ 122.658818] [run ]--- testing specific lock 8 failed (-14) --- > > [ 122.664658] [test] testing lock 9 > > [ 122.667971] [test]+++ attempt #0 succeded > > [ 122.671989] [test]+++ attempt #1 succeded > > > > Test 2: > > This is a more complex test that uses the status register to bypass the > > Linux hwspinlock ABI. For this to work a slightly modified driver is > > used and can be found here > > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/ > > modified_sunxi_hwspinlock > > 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 > > module which can be found here > > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test2_module > > It is also necessary to change the device tree entries to get both > > kernel modules working in parallel. > > > > hwspinlock-mod@1c18000 { > > compatible = "allwinner,sun50i-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,sun50i-hwspinlock-stat"; > > reg = <0x01c18010 0x4>; > > status = "okay"; > > }; > > > > The extended test kernel module supports 4 different modes of the test, > > 2 of them are sufficient to show the spinlock mechanism working. > > > > Mode 1: > > This one reads and prints the status register continuously. The crust > > firmware and the test are set to a hwlock timeout of 1 second. The test > > kernel module code runs a bit slower because of more code executed and > > you can see how one lock is missed between entry 2 and 3. > > > > > modprobe sunxi_hwspinlock_test2 mode=1 loops=10 > > [ 187.167074] [init]--- SUNXI HWSPINLOCK DRIVER TEST --- > > [ 187.172636] [sreg] 00000000_00010000_00000000_00000000 > > [ 188.196287] [sreg] 00000000_00001000_00000000_00000000 > > [ 189.220285] [sreg] 00000000_00000010_00000000_00000000 > > [ 190.244286] [sreg] 00000000_00000001_00000000_00000000 > > [ 191.268285] [sreg] 00000000_00000000_10000000_00000000 > > [ 192.292294] [sreg] 00000000_00000000_01000000_00000000 > > [ 193.316302] [sreg] 00000000_00000000_00100000_00000000 > > [ 194.340285] [sreg] 00000000_00000000_00010000_00000000 > > [ 195.364285] [sreg] 00000000_00000000_00001000_00000000 > > [ 196.388284] [sreg] 00000000_00000000_00000100_00000000 > > > > Mode 3: > > This mode combines the Linux hwspinlock ABI approach from test 1 and the > > status register access. The "after" reads show the locks taken by the > > Linux driver and the crust firmware. > > > > [ 439.138476] [test] testing lock 13 > > [ 439.141894] [sreg] before take 00000000_00000100_00000000_00000000 > > [ 439.148083] [sreg] after take 00000000_00000110_00000000_00000000 > > [ 439.154189] [sreg] after recursive take 00000000_00000110_00000000_00000000 > > [ 439.161162] [sreg] after untake 00000000_00000010_00000000_00000000 > > [ 439.167435] [test]+++ attempt #0 succeded > > [ 439.171458] [sreg] before take 00000000_00000010_00000000_00000000 > > [ 439.177649] [sreg] after take 00000000_00000110_00000000_00000000 > > [ 439.183751] [sreg] after recursive take 00000000_00000110_00000000_00000000 > > [ 439.190725] [sreg] after untake 00000000_00000010_00000000_00000000 > > [ 439.196992] [test]+++ attempt #1 succeded > > [ 439.201018] [sreg] before take 00000000_00000010_00000000_00000000 > > [ 439.207212] [sreg] after take 00000000_00000110_00000000_00000000 > > [ 439.213322] [sreg] after recursive take 00000000_00000110_00000000_00000000 > > [ 439.220290] [sreg] after untake 00000000_00000010_00000000_00000000 > > [ 439.226559] [test]+++ attempt #2 succeded > > [ 439.230576] [test] testing lock 14 > > [ 439.233996] [sreg] before take 00000000_00000010_00000000_00000000 > > [ 439.240177] [test] taking lock attempt #0 failed (-16) > > [ 439.245322] [run ]--- testing specific lock 14 failed (-14) --- > > [ 439.251252] [test] testing lock 15 > > [ 439.254677] [sreg] before take 00000000_00000010_00000000_00000000 > > [ 439.260865] [sreg] after take 00000000_00000011_00000000_00000000 > > [ 439.266975] [sreg] after recursive take 00000000_00000011_00000000_00000000 > > [ 439.273943] [sreg] after untake 00000000_00000010_00000000_00000000 > > [ 439.280212] [test]+++ attempt #0 succeded > > [ 439.284235] [sreg] before take 00000000_00000010_00000000_00000000 > > [ 439.290428] [sreg] after take 00000000_00000011_00000000_00000000 > > [ 439.296534] [sreg] after recursive take 00000000_00000011_00000000_00000000 > > [ 439.303502] [sreg] after untake 00000000_00000010_00000000_00000000 > > [ 439.309774] [test]+++ attempt #1 succeded > > Most of this should be in the cover letter (the details on how to make the tests mostly). > > However, I mentioned in your first version that some comments on the > previous drivers posted still applied to you, and you missed those > comments apparently. See here for more details: > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20200210170143.20007-2-nborisov@suse.com/#23148161 > > Most importantly, the driver name and compatible need to be changed. Hmm, so I basically could use sun8i or sun50i. But picking sun8i would be the logical one, because there it appeared first and sun50i is backwards compatible... What do you think? > Maxime ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] hwspinlock: add sunxi hardware spinlock support 2020-12-04 17:07 ` Wilken Gottwalt @ 2020-12-07 9:26 ` Maxime Ripard 0 siblings, 0 replies; 7+ messages in thread From: Maxime Ripard @ 2020-12-07 9:26 UTC (permalink / raw) To: Wilken Gottwalt Cc: linux-kernel, Ohad Ben-Cohen, Bjorn Andersson, Baolin Wang, Rob Herring, Chen-Yu Tsai, Jernej Skrabec [-- Attachment #1: Type: text/plain, Size: 11424 bytes --] On Fri, Dec 04, 2020 at 06:07:25PM +0100, Wilken Gottwalt wrote: > On Fri, 4 Dec 2020 17:21:17 +0100 > Maxime Ripard <maxime@cerno.tech> wrote: > > > On Fri, Dec 04, 2020 at 04:35:12PM +0100, Wilken Gottwalt wrote: > > > Adds the sunxi_hwspinlock driver for the hardware spinlock unit found in > > > most of the sun8i and sun50i based SoCs. > > > > > > This unit provides at least 32 spinlocks in hardware. The implementation > > > supports 32, 64, 128 or 256 32bit registers, where one 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 > > > currently 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 the 32 first locks only. For > > > this reason the lock read/write approach is used in this driver which > > > can cover all 256 locks in the biggest setup. > > > > > > Being able to use the status register to get knowledge about the locks > > > makes it suitable to extended testing. It also can be used to bypass > > > the Linux hwspinlock ABI completely and is not able to set locks. This > > > is used in one of the tests to show that the driver works correctly. > > > > > > 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 crust. In this case a H5 based device works very well. In > > > this test a Friendlyarm NanoPi NEO2 was used, which is fully supported > > > by u-boot (also the fork) 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, which uses the timeout functions of the crust > > > firmware. It loops through the first 32 spinlocks and takes/releases a > > > lock one after another using a timeout which can be set in the debug > > > menu. > > > > > > Test 1: > > > This test was done with a mainline u-boot and a crust enabled u-boot. > > > For this a simple second kernel module was used, found here > > > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test_module. > > > > > > If run with 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. Done for all 32 locks several times. > > > > > > [ 122.518435] [init]--- SUNXI HWSPINLOCK DRIVER TEST --- > > > [ 122.523810] [run ]--- testing locks 0 to 31 --- > > > [ 122.528370] [test] testing lock 0 > > > [ 122.531686] [test]+++ attempt #0 succeded > > > [ 122.535709] [test]+++ attempt #1 succeded > > > [ 122.539729] [test]+++ attempt #2 succeded > > > [ 122.543752] [test] testing lock 1 > > > [ 122.547072] [test]+++ attempt #0 succeded > > > [ 122.551093] [test]+++ attempt #1 succeded > > > [ 122.555113] [test]+++ attempt #2 succeded > > > [ 122.559129] [test] testing lock 2 > > > ... > > > [ 123.004812] [test] testing lock 31 > > > [ 123.008208] [test]+++ attempt #0 succeded > > > [ 123.012228] [test]+++ attempt #1 succeded > > > [ 123.016250] [test]+++ attempt #2 succeded > > > > > > Same test run with the hwspinlock test loop enabled in crust. Here the > > > test module hits two locks exactly in the moment crust switches from > > > lock 7 to 8. The test module code path is slower because of more code > > > that needs to be executed which makes this rare scenario possible. > > > > > > [ 122.620656] [test] testing lock 6 > > > [ 122.623966] [test]+++ attempt #0 succeded > > > [ 122.627988] [test]+++ attempt #1 succeded > > > [ 122.632008] [test]+++ attempt #2 succeded > > > [ 122.636044] [test] testing lock 7 > > > [ 122.639364] [test] taking lock attempt #0 failed (-16) > > > [ 122.644507] [run ]--- testing specific lock 7 failed (-14) --- > > > [ 122.650345] [test] testing lock 8 > > > [ 122.653671] [test] taking lock attempt #0 failed (-16) > > > [ 122.658818] [run ]--- testing specific lock 8 failed (-14) --- > > > [ 122.664658] [test] testing lock 9 > > > [ 122.667971] [test]+++ attempt #0 succeded > > > [ 122.671989] [test]+++ attempt #1 succeded > > > > > > Test 2: > > > This is a more complex test that uses the status register to bypass the > > > Linux hwspinlock ABI. For this to work a slightly modified driver is > > > used and can be found here > > > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/ > > > modified_sunxi_hwspinlock > > > 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 > > > module which can be found here > > > https://github.com/wgottwalt/sunxi_hwspinlock/tree/main/test2_module > > > It is also necessary to change the device tree entries to get both > > > kernel modules working in parallel. > > > > > > hwspinlock-mod@1c18000 { > > > compatible = "allwinner,sun50i-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,sun50i-hwspinlock-stat"; > > > reg = <0x01c18010 0x4>; > > > status = "okay"; > > > }; > > > > > > The extended test kernel module supports 4 different modes of the test, > > > 2 of them are sufficient to show the spinlock mechanism working. > > > > > > Mode 1: > > > This one reads and prints the status register continuously. The crust > > > firmware and the test are set to a hwlock timeout of 1 second. The test > > > kernel module code runs a bit slower because of more code executed and > > > you can see how one lock is missed between entry 2 and 3. > > > > > > > modprobe sunxi_hwspinlock_test2 mode=1 loops=10 > > > [ 187.167074] [init]--- SUNXI HWSPINLOCK DRIVER TEST --- > > > [ 187.172636] [sreg] 00000000_00010000_00000000_00000000 > > > [ 188.196287] [sreg] 00000000_00001000_00000000_00000000 > > > [ 189.220285] [sreg] 00000000_00000010_00000000_00000000 > > > [ 190.244286] [sreg] 00000000_00000001_00000000_00000000 > > > [ 191.268285] [sreg] 00000000_00000000_10000000_00000000 > > > [ 192.292294] [sreg] 00000000_00000000_01000000_00000000 > > > [ 193.316302] [sreg] 00000000_00000000_00100000_00000000 > > > [ 194.340285] [sreg] 00000000_00000000_00010000_00000000 > > > [ 195.364285] [sreg] 00000000_00000000_00001000_00000000 > > > [ 196.388284] [sreg] 00000000_00000000_00000100_00000000 > > > > > > Mode 3: > > > This mode combines the Linux hwspinlock ABI approach from test 1 and the > > > status register access. The "after" reads show the locks taken by the > > > Linux driver and the crust firmware. > > > > > > [ 439.138476] [test] testing lock 13 > > > [ 439.141894] [sreg] before take 00000000_00000100_00000000_00000000 > > > [ 439.148083] [sreg] after take 00000000_00000110_00000000_00000000 > > > [ 439.154189] [sreg] after recursive take 00000000_00000110_00000000_00000000 > > > [ 439.161162] [sreg] after untake 00000000_00000010_00000000_00000000 > > > [ 439.167435] [test]+++ attempt #0 succeded > > > [ 439.171458] [sreg] before take 00000000_00000010_00000000_00000000 > > > [ 439.177649] [sreg] after take 00000000_00000110_00000000_00000000 > > > [ 439.183751] [sreg] after recursive take 00000000_00000110_00000000_00000000 > > > [ 439.190725] [sreg] after untake 00000000_00000010_00000000_00000000 > > > [ 439.196992] [test]+++ attempt #1 succeded > > > [ 439.201018] [sreg] before take 00000000_00000010_00000000_00000000 > > > [ 439.207212] [sreg] after take 00000000_00000110_00000000_00000000 > > > [ 439.213322] [sreg] after recursive take 00000000_00000110_00000000_00000000 > > > [ 439.220290] [sreg] after untake 00000000_00000010_00000000_00000000 > > > [ 439.226559] [test]+++ attempt #2 succeded > > > [ 439.230576] [test] testing lock 14 > > > [ 439.233996] [sreg] before take 00000000_00000010_00000000_00000000 > > > [ 439.240177] [test] taking lock attempt #0 failed (-16) > > > [ 439.245322] [run ]--- testing specific lock 14 failed (-14) --- > > > [ 439.251252] [test] testing lock 15 > > > [ 439.254677] [sreg] before take 00000000_00000010_00000000_00000000 > > > [ 439.260865] [sreg] after take 00000000_00000011_00000000_00000000 > > > [ 439.266975] [sreg] after recursive take 00000000_00000011_00000000_00000000 > > > [ 439.273943] [sreg] after untake 00000000_00000010_00000000_00000000 > > > [ 439.280212] [test]+++ attempt #0 succeded > > > [ 439.284235] [sreg] before take 00000000_00000010_00000000_00000000 > > > [ 439.290428] [sreg] after take 00000000_00000011_00000000_00000000 > > > [ 439.296534] [sreg] after recursive take 00000000_00000011_00000000_00000000 > > > [ 439.303502] [sreg] after untake 00000000_00000010_00000000_00000000 > > > [ 439.309774] [test]+++ attempt #1 succeded > > > > Most of this should be in the cover letter (the details on how to make the tests mostly). > > > > However, I mentioned in your first version that some comments on the > > previous drivers posted still applied to you, and you missed those > > comments apparently. See here for more details: > > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20200210170143.20007-2-nborisov@suse.com/#23148161 > > > > Most importantly, the driver name and compatible need to be changed. > > Hmm, so I basically could use sun8i or sun50i. But picking sun8i would be > the logical one, because there it appeared first and sun50i is backwards > compatible... What do you think? Yep, for the driver names and compatibles we go for the earliest SoC/family that had that design. The 64 bits SoC came after the A33, so sun8i it is :) (And yes, your tests are fine by me, thanks!) Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-12-07 9:27 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-04 15:33 [PATCH v2 0/2] hwspinlock: add sunxi hardware spinlock support Wilken Gottwalt 2020-12-04 15:34 ` [PATCH v2 1/2] dt-bindings: hwlock: sunxi: add sunxi_hwspinlock documentation Wilken Gottwalt 2020-12-04 15:35 ` [PATCH v2 2/2] hwspinlock: add sunxi hardware spinlock support Wilken Gottwalt 2020-12-04 16:21 ` Maxime Ripard 2020-12-04 16:56 ` Wilken Gottwalt 2020-12-04 17:07 ` Wilken Gottwalt 2020-12-07 9:26 ` Maxime Ripard
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).