Skip to contentSkip to conversation

hwspinlock: qcom: Add support for mmio usage to sfpb-mutex

PATCH Superseded Christian Marangi 4 participants 12 messages Jul 7, 2022

Subsystems (3)
Device tree bindings Arm Qualcomm mailing list Hardware spinlock core

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Christian Marangi

author
#0 lore raw

PATCH 1/3 hwspinlock: qcom: Add support for mmio usage to sfpb-mutex

Allow sfpb-mutex to use mmio in addition to syscon.

Signed-off-by Christian Marangi <[email protected]>
---
 drivers/hwspinlock/qcom_hwspinlock.c | 32 ++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 7 deletions(-)
drivers/hwspinlock/qcom_hwspinlock.c +25 -7
index 364710966665..23c913095bd0 100644
@@ -19,6 +19,11 @@
 #define QCOM_MUTEX_APPS_PROC_ID	1
 #define QCOM_MUTEX_NUM_LOCKS	32
 
+struct qcom_hwspinlock_of_data {
+	u32 offset;
+	u32 stride;
+};
+
 static int qcom_hwspinlock_trylock(struct hwspinlock *lock)
 {
 	struct regmap_field *field = lock->priv;

@@ -63,9 +68,20 @@ static const struct hwspinlock_ops qcom_hwspinlock_ops = {
 	.unlock		= qcom_hwspinlock_unlock,
 };
 
+static const struct qcom_hwspinlock_of_data of_sfpb_mutex = {
+	.offset = 0x4,
+	.stride = 0x4,
+};
+
+/* All modern platform has offset 0 and stride of 4k */
+static const struct qcom_hwspinlock_of_data of_tcsr_mutex = {
+	.offset = 0,
+	.stride = 0x1000,
+};
+
 static const struct of_device_id qcom_hwspinlock_of_match[] = {
-	{ .compatible = "qcom,sfpb-mutex" },
-	{ .compatible = "qcom,tcsr-mutex" },
+	{ .compatible = "qcom,sfpb-mutex", .data = &of_sfpb_mutex },
+	{ .compatible = "qcom,tcsr-mutex", .data = &of_tcsr_mutex },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, qcom_hwspinlock_of_match);

@@ -101,7 +117,7 @@ static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev,
 	return regmap;
 }
 
-static const struct regmap_config tcsr_mutex_config = {
+static const struct regmap_config qcom_hwspinlock_mmio_config = {
 	.reg_bits		= 32,
 	.reg_stride		= 4,
 	.val_bits		= 32,

@@ -112,18 +128,20 @@ static const struct regmap_config tcsr_mutex_config = {
 static struct regmap *qcom_hwspinlock_probe_mmio(struct platform_device *pdev,
 						 u32 *offset, u32 *stride)
 {
+	const struct qcom_hwspinlock_of_data *data;
 	struct device *dev = &pdev->dev;
 	void __iomem *base;
 
-	/* All modern platform has offset 0 and stride of 4k */
-	*offset = 0;
-	*stride = 0x1000;
+	data = of_device_get_match_data(dev);
+
+	*offset = data->offset;
+	*stride = data->stride;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return ERR_CAST(base);
 
-	return devm_regmap_init_mmio(dev, base, &tcsr_mutex_config);
+	return devm_regmap_init_mmio(dev, base, &qcom_hwspinlock_mmio_config);
 }
 
 static int qcom_hwspinlock_probe(struct platform_device *pdev)
Signature · 1 line
2.36.1
author
#1 lore raw

PATCH 2/3 ARM: dts: qcom: add missing hwlock for ipq8064 dtsi

Add missing hwlock for ipq8064 dtsi provided by qcom,sfpb-mutex.

Signed-off-by Christian Marangi <[email protected]>
---
 arch/arm/boot/dts/qcom-ipq8064.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/boot/dts/qcom-ipq8064.dtsi +7 -0
index 4b475d98343c..cd1b43e2cab4 100644
@@ -1224,4 +1224,11 @@ sdcc3: mmc@12180000 {
 			};
 		};
 	};
+
+	sfpb_mutex: hwlock@1200600 {
+		compatible = "qcom,sfpb-mutex";
+		reg = <0x01200600 0x100>;
+
+		#hwlock-cells = <1>;
+	};
 };
Signature · 1 line
2.36.1
author
#3 lore raw

PATCH 3/3 ARM: dts: qcom: add missing smem compatible for ipq8064 dtsi

In reply to Christian Marangi

Add missing smem compatible and hwlocks binding for ipq8064 dtsi smem node.

Signed-off-by Christian Marangi <[email protected]>
---
 arch/arm/boot/dts/qcom-ipq8064.dtsi | 3 +++
 1 file changed, 3 insertions(+)
arch/arm/boot/dts/qcom-ipq8064.dtsi +3 -0
index cd1b43e2cab4..ff1a2acb1d83 100644
@@ -292,8 +292,11 @@ nss@40000000 {
 		};
 
 		smem: smem@41000000 {
+			compatible = "qcom,smem";
 			reg = <0x41000000 0x200000>;
 			no-map;
+
+			hwlocks = <&sfpb_mutex 3>;
 		};
 	};
Signature · 1 line
2.36.1
#5 lore raw

Re: [PATCH 3/3] ARM: dts: qcom: add missing smem compatible for ipq8064 dtsi

In reply to Christian Marangi
Quoted 1 line · Christian Marangi

Add missing smem compatible and hwlocks binding for ipq8064 dtsi

s/binding/phandle/

Best regards,
Krzysztof

author
#6 lore raw

PATCH 3/3 ARM: dts: qcom: add missing smem node for ipq8064 dtsi

In reply to Christian Marangi

Add missing smem node for ipq8064 dtsi.

Signed-off-by Christian Marangi <[email protected]>
---
 arch/arm/boot/dts/qcom-ipq8064.dtsi | 3 +++
 1 file changed, 3 insertions(+)
arch/arm/boot/dts/qcom-ipq8064.dtsi +3 -0
index cd1b43e2cab4..ff1a2acb1d83 100644
@@ -292,8 +292,11 @@ nss@40000000 {
 		};
 
 		smem: smem@41000000 {
+			compatible = "qcom,smem";
 			reg = <0x41000000 0x200000>;
 			no-map;
+
+			hwlocks = <&sfpb_mutex 3>;
 		};
 	};
Signature · 1 line
2.36.1
author
#7 lore raw

Re: [PATCH 3/3] ARM: dts: qcom: add missing smem node for ipq8064 dtsi

Quoted 2 lines · Christian Marangi

Add missing smem node for ipq8064 dtsi.

Slipped while fixing commit description... pls ignore.

Signature · 1 line
	Ansuel
Reviewed-by on v1 1/3
#8 lore raw
In reply to Christian Marangi
Quoted 2 lines · Christian Marangi

Allow sfpb-mutex to use mmio in addition to syscon.

Reviewed-by Bjorn Andersson <[email protected]>
Quoted 45 lines
Signed-off-by Christian Marangi <[email protected]>
---
 drivers/hwspinlock/qcom_hwspinlock.c | 32 ++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 7 deletions(-)
drivers/hwspinlock/qcom_hwspinlock.c +18 -2
index 364710966665..23c913095bd0 100644
@@ -19,6 +19,11 @@
 #define QCOM_MUTEX_APPS_PROC_ID	1
 #define QCOM_MUTEX_NUM_LOCKS	32
 
+struct qcom_hwspinlock_of_data {
+	u32 offset;
+	u32 stride;
+};
+
 static int qcom_hwspinlock_trylock(struct hwspinlock *lock)
 {
 	struct regmap_field *field = lock->priv;

@@ -63,9 +68,20 @@ static const struct hwspinlock_ops qcom_hwspinlock_ops = {
 	.unlock		= qcom_hwspinlock_unlock,
 };
 
+static const struct qcom_hwspinlock_of_data of_sfpb_mutex = {
+	.offset = 0x4,
+	.stride = 0x4,
+};
+
+/* All modern platform has offset 0 and stride of 4k */
+static const struct qcom_hwspinlock_of_data of_tcsr_mutex = {
+	.offset = 0,
+	.stride = 0x1000,
+};
+
 static const struct of_device_id qcom_hwspinlock_of_match[] = {
-	{ .compatible = "qcom,sfpb-mutex" },
-	{ .compatible = "qcom,tcsr-mutex" },
+	{ .compatible = "qcom,sfpb-mutex", .data = &of_sfpb_mutex },
+	{ .compatible = "qcom,tcsr-mutex", .data = &of_tcsr_mutex },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, qcom_hwspinlock_of_match);

drivers/hwspinlock/qcom_hwspinlock.c static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev, · line 120

Quoted 5 lines
drivers/hwspinlock/qcom_hwspinlock.c +1 -1
@@ -101,7 +117,7 @@ static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev,
 	return regmap;
 }
 
-static const struct regmap_config tcsr_mutex_config = {
+static const struct regmap_config qcom_hwspinlock_mmio_config = {

Any objections against me skipping this rename when applying the patch? Just to keep the patch to the point?

Regards,
Bjorn

Quoted 31 lines
drivers/hwspinlock/qcom_hwspinlock.c +6 -4
 	.reg_bits		= 32,
 	.reg_stride		= 4,
 	.val_bits		= 32,

@@ -112,18 +128,20 @@ static const struct regmap_config tcsr_mutex_config = {
 static struct regmap *qcom_hwspinlock_probe_mmio(struct platform_device *pdev,
 						 u32 *offset, u32 *stride)
 {
+	const struct qcom_hwspinlock_of_data *data;
 	struct device *dev = &pdev->dev;
 	void __iomem *base;
 
-	/* All modern platform has offset 0 and stride of 4k */
-	*offset = 0;
-	*stride = 0x1000;
+	data = of_device_get_match_data(dev);
+
+	*offset = data->offset;
+	*stride = data->stride;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return ERR_CAST(base);
 
-	return devm_regmap_init_mmio(dev, base, &tcsr_mutex_config);
+	return devm_regmap_init_mmio(dev, base, &qcom_hwspinlock_mmio_config);
 }
 
 static int qcom_hwspinlock_probe(struct platform_device *pdev)
Signature · 1 line
2.36.1
author
#9 lore raw
Quoted 55 lines · Bjorn Andersson
Quoted 2 lines · Christian Marangi

Allow sfpb-mutex to use mmio in addition to syscon.

Reviewed-by Bjorn Andersson <[email protected]>
Quoted 48 lines
Signed-off-by Christian Marangi <[email protected]>
---
 drivers/hwspinlock/qcom_hwspinlock.c | 32 ++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 7 deletions(-)
drivers/hwspinlock/qcom_hwspinlock.c +18 -2
index 364710966665..23c913095bd0 100644
@@ -19,6 +19,11 @@
 #define QCOM_MUTEX_APPS_PROC_ID	1
 #define QCOM_MUTEX_NUM_LOCKS	32
 
+struct qcom_hwspinlock_of_data {
+	u32 offset;
+	u32 stride;
+};
+
 static int qcom_hwspinlock_trylock(struct hwspinlock *lock)
 {
 	struct regmap_field *field = lock->priv;

@@ -63,9 +68,20 @@ static const struct hwspinlock_ops qcom_hwspinlock_ops = {
 	.unlock		= qcom_hwspinlock_unlock,
 };
 
+static const struct qcom_hwspinlock_of_data of_sfpb_mutex = {
+	.offset = 0x4,
+	.stride = 0x4,
+};
+
+/* All modern platform has offset 0 and stride of 4k */
+static const struct qcom_hwspinlock_of_data of_tcsr_mutex = {
+	.offset = 0,
+	.stride = 0x1000,
+};
+
 static const struct of_device_id qcom_hwspinlock_of_match[] = {
-	{ .compatible = "qcom,sfpb-mutex" },
-	{ .compatible = "qcom,tcsr-mutex" },
+	{ .compatible = "qcom,sfpb-mutex", .data = &of_sfpb_mutex },
+	{ .compatible = "qcom,tcsr-mutex", .data = &of_tcsr_mutex },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, qcom_hwspinlock_of_match);

@@ -101,7 +117,7 @@ static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev,
 	return regmap;
 }
Quoted 9 lines

drivers/hwspinlock/qcom_hwspinlock.c static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev, · line 120

Quoted 2 lines
drivers/hwspinlock/qcom_hwspinlock.c +1 -1
-static const struct regmap_config tcsr_mutex_config = {
+static const struct regmap_config qcom_hwspinlock_mmio_config = {

Any objections against me skipping this rename when applying the patch? Just to keep the patch to the point?

Regards,
Bjorn

I will send v2 with the name reverted and another fix.

Quoted 31 lines
Quoted 31 lines
drivers/hwspinlock/qcom_hwspinlock.c +6 -4
 	.reg_bits		= 32,
 	.reg_stride		= 4,
 	.val_bits		= 32,

@@ -112,18 +128,20 @@ static const struct regmap_config tcsr_mutex_config = {
 static struct regmap *qcom_hwspinlock_probe_mmio(struct platform_device *pdev,
 						 u32 *offset, u32 *stride)
 {
+	const struct qcom_hwspinlock_of_data *data;
 	struct device *dev = &pdev->dev;
 	void __iomem *base;
 
-	/* All modern platform has offset 0 and stride of 4k */
-	*offset = 0;
-	*stride = 0x1000;
+	data = of_device_get_match_data(dev);
+
+	*offset = data->offset;
+	*stride = data->stride;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return ERR_CAST(base);
 
-	return devm_regmap_init_mmio(dev, base, &tcsr_mutex_config);
+	return devm_regmap_init_mmio(dev, base, &qcom_hwspinlock_mmio_config);
 }
 
 static int qcom_hwspinlock_probe(struct platform_device *pdev)
Signature · 1 line
2.36.1
Signature · 1 line
	Ansuel
reviewer Reviewed-by on v1 1/3
#10 lore raw
In reply to Christian Marangi
Quoted 58 lines · Christian Marangi

Allow sfpb-mutex to use mmio in addition to syscon.

Signed-off-by Christian Marangi <[email protected]>
---
 drivers/hwspinlock/qcom_hwspinlock.c | 32 ++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 7 deletions(-)
drivers/hwspinlock/qcom_hwspinlock.c +19 -3
index 364710966665..23c913095bd0 100644
@@ -19,6 +19,11 @@
 #define QCOM_MUTEX_APPS_PROC_ID        1
 #define QCOM_MUTEX_NUM_LOCKS   32
 
+struct qcom_hwspinlock_of_data {
+       u32 offset;
+       u32 stride;
+};
+
 static int qcom_hwspinlock_trylock(struct hwspinlock *lock)
 {
        struct regmap_field *field = lock->priv;

@@ -63,9 +68,20 @@ static const struct hwspinlock_ops qcom_hwspinlock_ops = {
        .unlock         = qcom_hwspinlock_unlock,
 };
 
+static const struct qcom_hwspinlock_of_data of_sfpb_mutex = {
+       .offset = 0x4,
+       .stride = 0x4,
+};
+
+/* All modern platform has offset 0 and stride of 4k */
+static const struct qcom_hwspinlock_of_data of_tcsr_mutex = {
+       .offset = 0,
+       .stride = 0x1000,
+};
+
 static const struct of_device_id qcom_hwspinlock_of_match[] = {
-       { .compatible = "qcom,sfpb-mutex" },
-       { .compatible = "qcom,tcsr-mutex" },
+       { .compatible = "qcom,sfpb-mutex", .data = &of_sfpb_mutex },
+       { .compatible = "qcom,tcsr-mutex", .data = &of_tcsr_mutex },
        { }
 };
 MODULE_DEVICE_TABLE(of, qcom_hwspinlock_of_match);

@@ -101,7 +117,7 @@ static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev,
        return regmap;
 }
 
-static const struct regmap_config tcsr_mutex_config = {
+static const struct regmap_config qcom_hwspinlock_mmio_config = {
        .reg_bits               = 32,
        .reg_stride             = 4,
        .val_bits               = 32,

@@ -112,18 +128,20 @@ static const struct regmap_config tcsr_mutex_config = {
 static struct regmap *qcom_hwspinlock_probe_mmio(struct platform_device *pdev,

drivers/hwspinlock/qcom_hwspinlock.c static const struct regmap_config tcsr_mutex_config = { · line 135

Quoted 10 lines
drivers/hwspinlock/qcom_hwspinlock.c +2 -3
                                                 u32 *offset, u32 *stride)
 {
+       const struct qcom_hwspinlock_of_data *data;
        struct device *dev = &pdev->dev;
        void __iomem *base;
 
-       /* All modern platform has offset 0 and stride of 4k */
-       *offset = 0;
-       *stride = 0x1000;
+       data = of_device_get_match_data(dev);

Nit: better to validate the return value though this is a rare case.

if (!data)

         return -ENODEV;

Otherwise LGTM.

Reviewed-by Baolin Wang <[email protected]>
Signature · 1 line
Baolin Wang
author
#11 lore raw
Quoted 72 lines · Baolin Wang
Quoted 58 lines · Christian Marangi

Allow sfpb-mutex to use mmio in addition to syscon.

Signed-off-by Christian Marangi <[email protected]>
---
 drivers/hwspinlock/qcom_hwspinlock.c | 32 ++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 7 deletions(-)
drivers/hwspinlock/qcom_hwspinlock.c +19 -3
index 364710966665..23c913095bd0 100644
@@ -19,6 +19,11 @@
 #define QCOM_MUTEX_APPS_PROC_ID        1
 #define QCOM_MUTEX_NUM_LOCKS   32
 
+struct qcom_hwspinlock_of_data {
+       u32 offset;
+       u32 stride;
+};
+
 static int qcom_hwspinlock_trylock(struct hwspinlock *lock)
 {
        struct regmap_field *field = lock->priv;

@@ -63,9 +68,20 @@ static const struct hwspinlock_ops qcom_hwspinlock_ops = {
        .unlock         = qcom_hwspinlock_unlock,
 };
 
+static const struct qcom_hwspinlock_of_data of_sfpb_mutex = {
+       .offset = 0x4,
+       .stride = 0x4,
+};
+
+/* All modern platform has offset 0 and stride of 4k */
+static const struct qcom_hwspinlock_of_data of_tcsr_mutex = {
+       .offset = 0,
+       .stride = 0x1000,
+};
+
 static const struct of_device_id qcom_hwspinlock_of_match[] = {
-       { .compatible = "qcom,sfpb-mutex" },
-       { .compatible = "qcom,tcsr-mutex" },
+       { .compatible = "qcom,sfpb-mutex", .data = &of_sfpb_mutex },
+       { .compatible = "qcom,tcsr-mutex", .data = &of_tcsr_mutex },
        { }
 };
 MODULE_DEVICE_TABLE(of, qcom_hwspinlock_of_match);

@@ -101,7 +117,7 @@ static struct regmap *qcom_hwspinlock_probe_syscon(struct platform_device *pdev,
        return regmap;
 }
 
-static const struct regmap_config tcsr_mutex_config = {
+static const struct regmap_config qcom_hwspinlock_mmio_config = {
        .reg_bits               = 32,
        .reg_stride             = 4,
        .val_bits               = 32,

@@ -112,18 +128,20 @@ static const struct regmap_config tcsr_mutex_config = {
 static struct regmap *qcom_hwspinlock_probe_mmio(struct platform_device *pdev,

drivers/hwspinlock/qcom_hwspinlock.c static const struct regmap_config tcsr_mutex_config = { · line 135

Quoted 10 lines
drivers/hwspinlock/qcom_hwspinlock.c +2 -3
                                                 u32 *offset, u32 *stride)
 {
+       const struct qcom_hwspinlock_of_data *data;
        struct device *dev = &pdev->dev;
        void __iomem *base;
 
-       /* All modern platform has offset 0 and stride of 4k */
-       *offset = 0;
-       *stride = 0x1000;
+       data = of_device_get_match_data(dev);

Nit: better to validate the return value though this is a rare case.

Quoted 3 lines

if (!data)

         return -ENODEV;

Wonder if that can actually happen?

Looking at of_device_get_match_data() it can only return the data or NULL if the match data is not defined but considering it should ALWAYS be defined (or the driver can't work). But yhea should be a value to check.

Signature · 1 line
	Ansuel