[PATCH] soc: renesas: mfis: add hwspinlock support

[PATCH] soc: renesas: mfis: add hwspinlock support

[Wolfram Sang]

Every MFIS instance has a block of hardware spinlocks. Add support for
them. Gen4 has only one instance, so the base_id is always 0. Gen5 has
multiple instances, so the base_id has to be encoded in the info
description. Also being a provider, a hwspinlock-private header needs
to be included. Work to refactor the headers is on-going, but will need
many preparational steps. Until then, we need to live with the special
include.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

A branch for testing is here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/mfis/hwspinlock

Tested on SparrowHawk and Ironhide.

 drivers/soc/renesas/Kconfig     |  2 +-
 drivers/soc/renesas/rcar-mfis.c | 55 ++++++++++++++++++++++++++++++++-
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index 2ab150d04bb1..6c472c951ab3 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -468,7 +468,7 @@ endif # RISCV
 config RCAR_MFIS
 	tristate "Renesas R-Car MFIS driver"
 	depends on ARCH_RENESAS || COMPILE_TEST
-	depends on MAILBOX
+	depends on MAILBOX && HWSPINLOCK
 	help
 	  Select this option to enable the Renesas R-Car MFIS core driver for
 	  the MFIS device found on SoCs like R-Car. On families like Gen5, this
diff --git a/drivers/soc/renesas/rcar-mfis.c b/drivers/soc/renesas/rcar-mfis.c
index 3435c3e16198..a994b2cd6e59 100644
--- a/drivers/soc/renesas/rcar-mfis.c
+++ b/drivers/soc/renesas/rcar-mfis.c
@@ -8,6 +8,7 @@
  */
 #include <dt-bindings/soc/renesas,r8a78000-mfis.h>
 #include <linux/device.h>
+#include <linux/hwspinlock.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
@@ -19,12 +20,18 @@
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
 
+/* FIXME: hwspinlock core refactoring to not need internal header is on-going */
+#include "../../hwspinlock/hwspinlock_internal.h"
+
+#define MFISLCKR0	0xc0
+#define MFISLCKR8	0x0724
 #define MFISWPCNTR	0x0900
 #define MFISWACNTR	0x0904
 
 #define MFIS_X5H_IICR(i) ((i) * 0x1000 + 0x00)
 #define MFIS_X5H_EICR(i) ((i) * 0x1000 + 0x04)
 
+#define MFIS_NUM_LOCKS 64
 #define MFIS_UNPROTECT_KEY 0xACCE0000
 
 struct mfis_priv;
@@ -42,6 +49,7 @@ struct mfis_info {
 	unsigned int mb_tx_uses_eicr:1;
 	unsigned int mb_channels_are_unidir:1;
 	u32 (*mb_calc_reg)(u32 chan_num, bool tx_uses_eicr, bool is_only_rx);
+	int hwsp_base_id;
 };
 
 struct mfis_chan_priv {
@@ -59,6 +67,9 @@ struct mfis_priv {
 	/* mailbox private data */
 	struct mbox_controller mbox;
 	struct mfis_chan_priv *chan_privs;
+
+	/* hwspinlock private data */
+	struct hwspinlock_device bank;
 };
 
 static u32 mfis_read(struct mfis_reg *mreg, unsigned int reg)
@@ -86,6 +97,39 @@ static void mfis_write(struct mfis_reg *mreg, u32 reg, u32 val)
 	spin_unlock_irqrestore(&priv->unprotect_lock, flags);
 }
 
+/********************************************************
+ *			HW Spinlocks			*
+ ********************************************************/
+
+#define MFISLCKR8_CH_OFS (MFISLCKR8 - 8 * sizeof(u32))
+#define hwlock_to_local_id(hwlock) ((hwlock) - &(hwlock)->bank->lock[0])
+
+static int rcar_mfis_hwsp_trylock(struct hwspinlock *lock)
+{
+	struct mfis_priv *priv = lock->priv;
+	int id = hwlock_to_local_id(lock);
+	u32 val, reg;
+
+	reg = id * sizeof(u32) + (id < 8 ? MFISLCKR0 : MFISLCKR8_CH_OFS);
+	val = mfis_read(&priv->common_reg, reg);
+	return !val;
+}
+
+static void rcar_mfis_hwsp_unlock(struct hwspinlock *lock)
+{
+	struct mfis_priv *priv = lock->priv;
+	int id = hwlock_to_local_id(lock);
+	u32 reg;
+
+	reg = id * sizeof(u32) + (id < 8 ? MFISLCKR0 : MFISLCKR8_CH_OFS);
+	mfis_write(&priv->common_reg, reg, 0);
+}
+
+static const struct hwspinlock_ops rcar_mfis_hwsp_ops = {
+	.trylock	= rcar_mfis_hwsp_trylock,
+	.unlock		= rcar_mfis_hwsp_unlock,
+};
+
 /********************************************************
  *			Mailbox				*
  ********************************************************/
@@ -314,7 +358,7 @@ static int mfis_probe(struct platform_device *pdev)
 	struct mfis_priv *priv;
 	int ret;
 
-	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(dev, struct_size(priv, bank.lock, MFIS_NUM_LOCKS), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
@@ -333,6 +377,14 @@ static int mfis_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	for (unsigned int ch = 0; ch < MFIS_NUM_LOCKS; ch++)
+		priv->bank.lock[ch].priv = priv;
+
+	ret = devm_hwspin_lock_register(dev, &priv->bank, &rcar_mfis_hwsp_ops,
+					priv->info->hwsp_base_id, MFIS_NUM_LOCKS);
+	if (ret)
+		return ret;
+
 	return mfis_mb_probe(priv);
 }
 
@@ -357,6 +409,7 @@ static const struct mfis_info mfis_info_r8a78000_scp = {
 	.mb_tx_uses_eicr = true,
 	.mb_channels_are_unidir = true,
 	.mb_calc_reg = mfis_mb_r8a78000_calc_reg,
+	.hwsp_base_id = MFIS_NUM_LOCKS,
 };
 
 static const struct of_device_id mfis_mfd_of_match[] = {
-- 
2.51.0

Re: [PATCH] soc: renesas: mfis: add hwspinlock support

[Wolfram Sang]

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


> [Severity: High]
> Will this sleep in atomic context on preempt-rt kernels?
> 
> The hwspinlock core can invoke unlock callbacks from raw atomic contexts
> (for example, via hwspin_unlock_in_atomic). However, this callback invokes
> mfis_write(), which acquires the unprotect_lock:
> 
> mfis_write()
>     spin_lock_irqsave(&priv->unprotect_lock, flags);
> 
> On preempt-rt builds, a spinlock_t becomes a preemptible, sleepable rt_mutex.
> Acquiring it inside an atomic context could lead to a sleeping in atomic
> regression. On standard kernels, it also violates the lockdep wait-type
> hierarchy since we are nesting a spinlock inside a raw atomic context.
> 
> Should unprotect_lock be converted to a raw_spinlock_t to avoid this
> regression?

The spinlock was a best effort thing at the time. It should be a
hwspinlock even to protect against accesses from non-AP cores. I thought
this could be postponed to later, but it seems it cannot.


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

Re: [PATCH] soc: renesas: mfis: add hwspinlock support

[Wolfram Sang]

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


> The spinlock was a best effort thing at the time. It should be a
> hwspinlock even to protect against accesses from non-AP cores. I thought
> this could be postponed to later, but it seems it cannot.

Then again, hwspinlock core seems to need more updates until we can
finally do that. I'll use the raw_spinlock as an intermediate step.


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