drivers: hwspinlock: sqn: use cluster id instead of core mpidr

Using the mpidr of the core to lock the hwspinlock meant it acted as
a spinlock in the cluster itself, which is exactly the opposite of what we
tried to achieve.
Additionally, this tied the driver to the arm architecture, even though the
HW was not.

Signed-off-by: Pierrick Guillaume <[email protected]>

Author: Fymyte

drivers/hwspinlock/Kconfig.sqn

@@ -11,6 +11,7 @@ config SQN_HWSPINLOCK
 	  Enable HW spinlock for SQN
 
 if SQN_HWSPINLOCK
+
 config SQN_HWSPINLOCK_RELAX_TIME
 	int "Sequans HW spinlock relax time"
 	default 50

drivers/hwspinlock/sqn_hwspinlock.c

@@ -1,19 +1,30 @@
 /*
- * Copyright (c) 2023 Sequans Communications
+ * Copyright (c) 2025 Sequans Communications
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 
 #define DT_DRV_COMPAT sqn_hwspinlock
 
-#include <zephyr/device.h>
 #include <zephyr/kernel.h>
 #include <zephyr/sys/sys_io.h>
+#include <zephyr/sys/__assert.h>
 #include <zephyr/drivers/hwspinlock.h>
 
-#include <zephyr/sys/printk.h>
 #include <zephyr/logging/log.h>
-LOG_MODULE_REGISTER(sqn_hwspinlock);
+LOG_MODULE_REGISTER(sqn_hwspinlock, CONFIG_HWSPINLOCK_INIT_PRIORITY);
+
+#include <soc.h>
+
+/*
+ * HWSPINLOCK mechanism:
+ * When a hwspinlock is unlocked, its register value is 0.
+ * To claim the spinlock, the user write its cluster id.
+ * To release it, the user write its cluster id again.
+ * Trying to write another value does not update the value of the register.
+ * To check that the user holds the lock, it reads the register and compare the value with its
+ * cluster id.
+ */
 
 struct sqn_hwspinlock_data {
 	DEVICE_MMIO_RAM;
@@ -24,45 +35,30 @@ struct sqn_hwspinlock_config {
 	uint32_t num_locks;
 };
 
-static inline mem_addr_t get_lock_addr(const struct device *dev, uint32_t id)
-{
-	return (mem_addr_t)(DEVICE_MMIO_GET(dev) + id * sizeof(uint32_t));
-}
+#define DEV_DATA(dev) ((struct sqn_hwspinlock_data *)dev->data)
+#define DEV_CFG(dev)  ((const struct sqn_hwspinlock_config *)dev->config)
 
 /*
- * To define CPU id, we use the affinity2 and affinity1
- * fields of the MPIDR register.
+ * CURRENT_CLUSTER value starts at 0. Because 0 is a reserved value for the register, increment all
+ * ids by 1.
  */
-static uint8_t mpidr_to_cpuid(uint64_t mpidr_val)
-{
-	uint8_t cpuid = ((mpidr_val >> 8) & 0x0F) | ((mpidr_val >> 12) & 0xF0);
+#define HWSPINLOCK_CLUSTER_ID (CURRENT_CLUSTER + 1)
 
-	return ++cpuid;
+static inline mem_addr_t get_lock_addr(const struct device *dev, uint32_t id)
+{
+	return (mem_addr_t)(DEVICE_MMIO_GET(dev) + id * sizeof(uint32_t));
 }
 
 static int sqn_hwspinlock_trylock(const struct device *dev, uint32_t id)
 {
-	const struct sqn_hwspinlock_config *config = dev->config;
-	uint8_t cpuid;
+	mem_addr_t lock_addr = get_lock_addr(dev, id);
 
-	if (id > config->num_locks) {
-		return -EINVAL;
-	}
-
-	/*
-	 * If the register value is equal to cpuid, this means that the current
-	 * core has already locked the HW spinlock.
-	 * If not, we try to lock the HW spinlock by writing cpuid, then check
-	 * whether it is locked.
-	 */
-
-	cpuid = mpidr_to_cpuid(read_mpidr_el1());
-	if (sys_read8(get_lock_addr(dev, id)) == cpuid) {
-		return 0;
-	}
+	__ASSERT(id < DEV_CFG(dev)->num_locks, "Invalid hwspinlock id");
+	__ASSERT(sys_read8(lock_addr) != HWSPINLOCK_CLUSTER_ID,
+		 "Tried to lock hwspinlock already locked by this cluster");
 
-	sys_write8(cpuid, get_lock_addr(dev, id));
-	if (sys_read8(get_lock_addr(dev, id)) == cpuid) {
+	sys_write8(HWSPINLOCK_CLUSTER_ID, lock_addr);
+	if (sys_read8(lock_addr) == HWSPINLOCK_CLUSTER_ID) {
 		return 0;
 	}
 
@@ -71,59 +67,37 @@ static int sqn_hwspinlock_trylock(const struct device *dev, uint32_t id)
 
 static void sqn_hwspinlock_lock(const struct device *dev, uint32_t id)
 {
-	const struct sqn_hwspinlock_config *config = dev->config;
-	uint8_t cpuid;
-
-	if (id > config->num_locks) {
-		LOG_ERR("unsupported hwspinlock id '%d'", id);
-		return;
-	}
-
-	/*
-	 * Writing cpuid is equivalent to trying to lock HW spinlock, after
-	 * which we check whether we've locked by reading the register value
-	 * and comparing it with cpuid.
-	 */
+	mem_addr_t lock_addr = get_lock_addr(dev, id);
 
-	cpuid = mpidr_to_cpuid(read_mpidr_el1());
-	if (sys_read8(get_lock_addr(dev, id)) == 0) {
-		sys_write8(cpuid, get_lock_addr(dev, id));
-	}
+	__ASSERT(id < DEV_CFG(dev)->num_locks, "Invalid hwspinlock id");
+	__ASSERT(sys_read8(lock_addr) != HWSPINLOCK_CLUSTER_ID,
+		 "Tried to lock hwspinlock already locked by this cluster");
 
-	while (sys_read8(get_lock_addr(dev, id)) != cpuid) {
+	sys_write8(HWSPINLOCK_CLUSTER_ID, lock_addr);
+	while (sys_read8(lock_addr) != HWSPINLOCK_CLUSTER_ID) {
 		k_busy_wait(CONFIG_SQN_HWSPINLOCK_RELAX_TIME);
-		sys_write8(cpuid, get_lock_addr(dev, id));
+		sys_write8(HWSPINLOCK_CLUSTER_ID, lock_addr);
 	}
 }
 
 static void sqn_hwspinlock_unlock(const struct device *dev, uint32_t id)
 {
-	const struct sqn_hwspinlock_config *config = dev->config;
-	uint8_t cpuid;
+	mem_addr_t lock_addr = get_lock_addr(dev, id);
 
-	if (id > config->num_locks) {
-		LOG_ERR("unsupported hwspinlock id '%d'", id);
-		return;
-	}
-
-	/*
-	 * If the HW spinlock register value is equal to the cpuid and we write
-	 * the cpuid, then the register value will be 0. So to unlock the
-	 * hwspinlock, we write cpuid.
-	 */
+	__ASSERT(id < DEV_CFG(dev)->num_locks, "Invalid hwspinlock id");
+	__ASSERT(sys_read8(lock_addr) == HWSPINLOCK_CLUSTER_ID,
+		 "Tried to unlock hwspinlock not locked by this cluster");
 
-	cpuid = mpidr_to_cpuid(read_mpidr_el1());
-	sys_write8(cpuid, get_lock_addr(dev, id));
+	/* Unlock by writing our own cluster id again */
+	sys_write8(HWSPINLOCK_CLUSTER_ID, lock_addr);
 }
 
 static uint32_t sqn_hwspinlock_get_max_id(const struct device *dev)
 {
-	const struct sqn_hwspinlock_config *config = dev->config;
-
-	return config->num_locks;
+	return DEV_CFG(dev)->num_locks;
 }
 
-static DEVICE_API(hwspinlock, hwspinlock_api) = {
+static DEVICE_API(hwspinlock, sqn_hwspinlock_api) = {
 	.trylock = sqn_hwspinlock_trylock,
 	.lock = sqn_hwspinlock_lock,
 	.unlock = sqn_hwspinlock_unlock,
@@ -137,18 +111,14 @@ static int sqn_hwspinlock_init(const struct device *dev)
 	return 0;
 }
 
-#define SQN_HWSPINLOCK_INIT(idx)                                                \
-	static struct sqn_hwspinlock_data sqn_hwspinlock##idx##_data;           \
-	static const struct sqn_hwspinlock_config sqn_hwspinlock##idx##_config = {    \
-		DEVICE_MMIO_ROM_INIT(DT_DRV_INST(idx)),                         \
-		.num_locks = DT_INST_PROP(idx, num_locks),                      \
-	};                                                                      \
-	DEVICE_DT_INST_DEFINE(idx,                                              \
-			      sqn_hwspinlock_init,                              \
-			      NULL,                                             \
-			      &sqn_hwspinlock##idx##_data,                      \
-			      &sqn_hwspinlock##idx##_config,                    \
-			      PRE_KERNEL_1, CONFIG_HWSPINLOCK_INIT_PRIORITY,    \
-			      &hwspinlock_api)
+#define SQN_HWSPINLOCK_INIT(idx)                                                                   \
+	static struct sqn_hwspinlock_data sqn_hwspinlock##idx##_data;                              \
+	static const struct sqn_hwspinlock_config sqn_hwspinlock##idx##_config = {                 \
+		DEVICE_MMIO_ROM_INIT(DT_DRV_INST(idx)),                                            \
+		.num_locks = DT_INST_PROP(idx, num_locks),                                         \
+	};                                                                                         \
+	DEVICE_DT_INST_DEFINE(idx, sqn_hwspinlock_init, NULL, &sqn_hwspinlock##idx##_data,         \
+			      &sqn_hwspinlock##idx##_config, PRE_KERNEL_1,                         \
+			      CONFIG_HWSPINLOCK_INIT_PRIORITY, &sqn_hwspinlock_api)
 
 DT_INST_FOREACH_STATUS_OKAY(SQN_HWSPINLOCK_INIT);

dts/bindings/hwspinlock/sqn,hwspinlock.yaml

@@ -1,11 +1,11 @@
-# Copyright (c) 2023 Sequans Communications
+# Copyright (c) 2025 Sequans Communications
 # SPDX-License-Identifier: Apache-2.0
 
 description: SQN Hardware spinlocks
 
 compatible: "sqn,hwspinlock"
 
-include: base.yaml
+include: [base.yaml, hwspinlock-controller.yaml]
 
 properties:
   reg:
@@ -14,3 +14,6 @@ properties:
   num-locks:
     type: int
     required: true
+
+hwlock-cells:
+  - id