Received: by 2002:a05:6a10:f347:0:0:0:0 with SMTP id d7csp483336pxu; Thu, 7 Jan 2021 09:46:13 -0800 (PST) X-Google-Smtp-Source: ABdhPJxbx5TXvuZR47/vtdnvjdZ8DjobMN4dhq3Z9RTv+hFXB6U0YLDPClOoHEIrNGsr/WWGJ0JU X-Received: by 2002:a17:906:73d8:: with SMTP id n24mr7037398ejl.14.1610041573315; Thu, 07 Jan 2021 09:46:13 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1610041573; cv=none; d=google.com; s=arc-20160816; b=MQjyREb6ETUg+HvOR+reqIJ6v9DIoaC8/wUeJ3RFAoR/JDqbg6mro2//b4bF9qYOUz BQWjOERrlG9rLUtEq8U/rvPod1TZI5F/+vyc/MFKfb4B7b6KPYSr9TBpYbb2xuSPe9dA kFxMAHYwAPtuiSMdtiewj1g2D4Jyfar9ey3ebM8wmTwjZ2pQQCgMIyuOuEtEIz0Kih8h 44ex0JGh78zQpY+nk2ecwC5bvqiPotwWNOBsB/j1watomhE4/MPe9kumwdyhvY7bV/sE Vm3QwN6t532Dj2pz/VvaQlEMS9wP70jKyh4wTsfqOY1HSRiDNIzmarK08psukM0SfnF1 JADQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=hVuCAmcENRT4YTWMqLA1a/iaC1560RLZ0bRkSPBMwf4=; b=B/j7fqqEPTzpdBZAhPpRVa4MSgixRuDrKwCg5AJcPQ8rBf492ot6wrfnBlPT457Isc 9AaGBtH0JZARZ3dUgUXupk8wWYO7TG1SjmbH3tRBSIPQ3QlZIy/xOAOMWArdvOSGML13 RWH1qHb5CAulhAJ7upaOxW5QOoB6Q1D2tazLDGlAC8zS82I4+i+amUze8LX2VJvGfTxN VKHDTMiztHxWsG04ucQrJK/Qlz7Ejsf31V/910rG39JnCs67xe/qiflnQo/jxwGwcKnW MOh6ZlffE2zqfuOgE8meclHK50Ijs5MblwzVThYYMkPJoW+n+8brPwzU4nWfCK7zZTQL tXQg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b="kTUsN/Dn"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id t5si2435489ejr.740.2021.01.07.09.45.49; Thu, 07 Jan 2021 09:46:13 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b="kTUsN/Dn"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726294AbhAGRob (ORCPT + 99 others); Thu, 7 Jan 2021 12:44:31 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58512 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725944AbhAGRob (ORCPT ); Thu, 7 Jan 2021 12:44:31 -0500 Received: from mail-oi1-x232.google.com (mail-oi1-x232.google.com [IPv6:2607:f8b0:4864:20::232]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 02F45C0612F4 for ; Thu, 7 Jan 2021 09:43:51 -0800 (PST) Received: by mail-oi1-x232.google.com with SMTP id l207so8253050oib.4 for ; Thu, 07 Jan 2021 09:43:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=hVuCAmcENRT4YTWMqLA1a/iaC1560RLZ0bRkSPBMwf4=; b=kTUsN/DncYwlb0Ym5/DHhoV4R1m9C+PCCR8Bd+9sbJsdTr3P/D1S9q90cKQIGGa9uu Dl2SYgORQubfRp18DSSvm7SVTtKWoJvpgMpT6ttJ1Hn4DqMlC75r+5TBFn9nk/mfvEOg X6wAWjlUu1M8IqptHNa9FwJTVauDs4O0ngmT9T5lu1BQRl8JA5tHXUiCTk57p6WZeCF6 4qhiiSSnU9FtFc2/u5LC3Ze5mSDoZWcKqiY7CLDny6NnssxmzkrDslhjfmQKa6dwEdb1 ladZcRh8oyVg55MTmRiruDarhhQ8TQMxMHj2qCD6+KxdWgMAV5Z49kzyWa6h6y9ljfIg XmfQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=hVuCAmcENRT4YTWMqLA1a/iaC1560RLZ0bRkSPBMwf4=; b=lrKtWr1sudTASOUWBEWW8F14lZef2MGVj7PttD2Tomp2S6RukAzQCVuYkH7XCoEBgO JitllMQw9UO3OAwx8L6LNiFG4MsaW+RTGFQidBxuVK+3Po6EcFD+bBhfx4e6n69uh2YK g81UqsFU1ZoX/HkRvDgmrZwEO2uMsJx2+U6FH20nTkI/4peGBZYU/xpWg/iIH1yNhYKa aJzuj1HdXuWEXwefhHjW8DNIgqCSazxk3Cb4VHX35S/AUHKPd74Kr4H6J22DzYAHhUzJ WTx8NGW9aeAs8NzGKLV72O8NI7FDy4y0/OQhfIdt4f4HLHuiJ2gzIipl4/eTfm8Zxyi9 gm7g== X-Gm-Message-State: AOAM532Wd0q6bo56jnvm8E6xCfeNxOEunhl6ZOKiCVlvjQXVNXb5Wo5N xZ8/3FSbJV/J4JvrQhJC2va6Zg== X-Received: by 2002:a05:6808:10b:: with SMTP id b11mr7508335oie.90.1610041430313; Thu, 07 Jan 2021 09:43:50 -0800 (PST) Received: from builder.lan (104-57-184-186.lightspeed.austtx.sbcglobal.net. [104.57.184.186]) by smtp.gmail.com with ESMTPSA id 186sm1425731oie.38.2021.01.07.09.43.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Jan 2021 09:43:49 -0800 (PST) Date: Thu, 7 Jan 2021 11:43:48 -0600 From: Bjorn Andersson To: Wilken Gottwalt Cc: linux-kernel@vger.kernel.org, Ohad Ben-Cohen , Baolin Wang , Rob Herring , Maxime Ripard , Chen-Yu Tsai , Jernej Skrabec Subject: Re: [PATCH v5 2/2] hwspinlock: add sun6i hardware spinlock support Message-ID: References: <0deae76aec31586da45c316546b12bcc316442ee.1608721968.git.wilken.gottwalt@posteo.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0deae76aec31586da45c316546b12bcc316442ee.1608721968.git.wilken.gottwalt@posteo.net> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed 23 Dec 05:35 CST 2020, Wilken Gottwalt wrote: > Adds the sun6i_hwspinlock driver for the hardware spinlock unit found in > most of the sun6i 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 companion 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 companion 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 > --- > Changes in v5: > - changed symbols to the earliest known supported SoC (sun6i/a31) > - changed init back to classic probe/remove callbacks > > Changes in v4: > - further simplified driver > - fixed an add_action_and_reset_ function issue > - changed bindings from sun8i-hwspinlock to sun8i-a33-hwspinlock > > Changes in v3: > - moved test description to cover letter > - changed name and symbols from sunxi to sun8i > - improved driver description > - further simplified 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/sun6i_hwspinlock.c | 214 ++++++++++++++++++++++++++ > 4 files changed, 230 insertions(+) > create mode 100644 drivers/hwspinlock/sun6i_hwspinlock.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index ad0e34bf8453..0842b2a3ea89 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 > +S: Maintained > +F: Documentation/devicetree/bindings/hwlock/sun6i-a31-hwspinlock.yaml > +F: drivers/hwspinlock/sun6i_hwspinlock.c > + > ALLWINNER THERMAL DRIVER > M: Vasily Khoruzhick > M: Yangtao Li > diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig > index 32cd26352f38..56ecc1aa3166 100644 > --- a/drivers/hwspinlock/Kconfig > +++ b/drivers/hwspinlock/Kconfig > @@ -55,6 +55,15 @@ config HWSPINLOCK_STM32 > > If unsure, say N. > > +config HWSPINLOCK_SUN6I > + tristate "SUN6I Hardware Spinlock device" > + depends on ARCH_SUNXI || COMPILE_TEST > + help > + Say y here to support the SUN6I Hardware Spinlock device which can be > + found in most of the sun6i 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..83ec4f03decc 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_SUN6I) += sun6i_hwspinlock.o > obj-$(CONFIG_HSEM_U8500) += u8500_hsem.o > diff --git a/drivers/hwspinlock/sun6i_hwspinlock.c b/drivers/hwspinlock/sun6i_hwspinlock.c > new file mode 100644 > index 000000000000..ba56eed818e7 > --- /dev/null > +++ b/drivers/hwspinlock/sun6i_hwspinlock.c > @@ -0,0 +1,214 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * sun6i_hwspinlock.c - hardware spinlock driver for sun6i compatible Allwinner SoCs > + * Copyright (C) 2020 Wilken Gottwalt > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "hwspinlock_internal.h" > + > +#define DRIVER_NAME "sun6i_hwspinlock" Isn't this the same as KBUILD_MODNAME? > + > +#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 sun6i_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 sun6i_hwspinlock_data *priv = seqf->private; > + > + seq_printf(seqf, "%d\n", priv->nlocks); > + > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(hwlocks_supported); > + > +static void sun6i_hwspinlock_debugfs_init(struct sun6i_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 sun6i_hwspinlock_debugfs_init(struct sun6i_hwspinlock_data *priv) > +{ > +} > + > +#endif > + > +static int sun6i_hwspinlock_trylock(struct hwspinlock *lock) > +{ > + void __iomem *lock_addr = lock->priv; > + > + return (readl(lock_addr) == SPINLOCK_NOTTAKEN); > +} > + > +static void sun6i_hwspinlock_unlock(struct hwspinlock *lock) > +{ > + void __iomem *lock_addr = lock->priv; > + > + writel(SPINLOCK_NOTTAKEN, lock_addr); > +} > + > +static const struct hwspinlock_ops sun6i_hwspinlock_ops = { > + .trylock = sun6i_hwspinlock_trylock, > + .unlock = sun6i_hwspinlock_unlock, > +}; > + > +static int sun6i_hwspinlock_probe(struct platform_device *pdev) > +{ > + struct sun6i_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)) > + return PTR_ERR(io_base); > + > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + 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(&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); > + goto clk_fail; > + } > + > + /* > + * bit 28 and 29 represents the hwspinlock setup > + * > + * every datasheet (A64, A80, A83T, H3, H5, H6 ...) says the default value is 0x1 and 0x1 > + * to 0x4 represent 32, 64, 128 and 256 locks > + * but later datasheets (H5, H6) say 00, 01, 10, 11 represent 32, 64, 128 and 256 locks, > + * but that would mean H5 and H6 have 64 locks, while their datasheets talk about 32 locks > + * all the time, not a single mentioning of 64 locks > + * the 0x4 value is also not representable by 2 bits alone, so some datasheets are not > + * correct > + * one thing have all in common, default value of the sysstatus register is 0x10000000, > + * which results in bit 28 being set > + * this is the reason 0x1 is considered being 32 locks and bit 30 is taken into account > + * verified on H2+ (datasheet 0x1 = 32 locks) and H5 (datasheet 01 = 64 locks) > + */ > + num_banks = readl(io_base + SPINLOCK_SYSSTATUS_REG) >> 28; > + switch (num_banks) { > + case 1 ... 4: > + priv->nlocks = 1 << (4 + num_banks); > + break; > + default: > + err = -EINVAL; > + dev_err(&pdev->dev, "unsupported hwspinlock setup (%d)\n", num_banks); > + goto bank_fail; > + } > + > + priv->bank = devm_kzalloc(&pdev->dev, struct_size(priv->bank, lock, priv->nlocks), > + GFP_KERNEL); > + if (!priv->bank) { > + err = -ENOMEM; > + goto bank_fail; > + } > + > + for (i = 0; i < priv->nlocks; ++i) { > + hwlock = &priv->bank->lock[i]; > + hwlock->priv = io_base + SPINLOCK_LOCK_REGN + sizeof(u32) * i; > + } > + > + /* failure of debugfs is considered non-fatal */ > + sun6i_hwspinlock_debugfs_init(priv); > + if (IS_ERR(priv->debugfs)) > + priv->debugfs = NULL; > + > + platform_set_drvdata(pdev, priv); > + > + return devm_hwspin_lock_register(&pdev->dev, priv->bank, &sun6i_hwspinlock_ops, > + SPINLOCK_BASE_ID, priv->nlocks); If this fails you will leave the reset deasserted and the clocks prepared. So please handle this as well. Regards, Bjorn > +bank_fail: > + clk_disable_unprepare(priv->ahb_clk); > +clk_fail: > + reset_control_assert(priv->reset); > + > + return err; > +} > + > +static int sun6i_hwspinlock_remove(struct platform_device *pdev) > +{ > + struct sun6i_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; > + } > + > + clk_disable_unprepare(priv->ahb_clk); > + reset_control_assert(priv->reset); > + > + return 0; > +} > + > +static const struct of_device_id sun6i_hwspinlock_ids[] = { > + { .compatible = "allwinner,sun6i-a31-hwspinlock", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, sun6i_hwspinlock_ids); > + > +static struct platform_driver sun6i_hwspinlock_driver = { > + .probe = sun6i_hwspinlock_probe, > + .remove = sun6i_hwspinlock_remove, > + .driver = { > + .name = DRIVER_NAME, > + .of_match_table = sun6i_hwspinlock_ids, > + }, > +}; > +module_platform_driver(sun6i_hwspinlock_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("SUN6I hardware spinlock driver"); > +MODULE_AUTHOR("Wilken Gottwalt "); > -- > 2.29.2 >