[PATCH 1/8] hwspinlock: Add one new API to support getting a specific hwlock by the name

[PATCH 1/8] hwspinlock: Add one new API to support getting a specific hwlock by the name

[Baolin Wang]

The hardware spinlock binding already supplied the 'hwlock-names' property
to match and get a specific hwlock, but did not supply one API for users
to get a specific hwlock by the hwlock name. So this patch introduces one
API to support this requirement.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/hwspinlock/hwspinlock_core.c |   29 +++++++++++++++++++++++++++++
 include/linux/hwspinlock.h           |    7 +++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
index d16e6a3..bea3586 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -367,6 +367,35 @@ int of_hwspin_lock_get_id(struct device_node *np, int index)
 }
 EXPORT_SYMBOL_GPL(of_hwspin_lock_get_id);
 
+/**
+ * of_hwspin_lock_get_id_byname() - get lock id for an specified hwlock name
+ * @np: device node from which to request the specific hwlock
+ * @name: hwlock name
+ *
+ * This function provides a means for DT users of the hwspinlock module to
+ * get the global lock id of a specific hwspinlock using the specified name of
+ * the hwspinlock device, so that it can be requested using the normal
+ * hwspin_lock_request_specific() API.
+ *
+ * Returns the global lock id number on success, -EPROBE_DEFER if the hwspinlock
+ * device is not yet registered, -EINVAL on invalid args specifier value or an
+ * appropriate error as returned from the OF parsing of the DT client node.
+ */
+int of_hwspin_lock_get_id_byname(struct device_node *np, const char *name)
+{
+	int index;
+
+	if (!name)
+		return -EINVAL;
+
+	index = of_property_match_string(np, "hwlock-names", name);
+	if (index < 0)
+		return index;
+
+	return of_hwspin_lock_get_id(np, index);
+}
+EXPORT_SYMBOL_GPL(of_hwspin_lock_get_id_byname);
+
 static int hwspin_lock_register_single(struct hwspinlock *hwlock, int id)
 {
 	struct hwspinlock *tmp;
diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
index 57537e6..2b6f389 100644
--- a/include/linux/hwspinlock.h
+++ b/include/linux/hwspinlock.h
@@ -66,6 +66,7 @@ int __hwspin_lock_timeout(struct hwspinlock *, unsigned int, int,
 							unsigned long *);
 int __hwspin_trylock(struct hwspinlock *, int, unsigned long *);
 void __hwspin_unlock(struct hwspinlock *, int, unsigned long *);
+int of_hwspin_lock_get_id_byname(struct device_node *np, const char *name);
 
 #else /* !CONFIG_HWSPINLOCK */
 
@@ -125,6 +126,12 @@ static inline int hwspin_lock_get_id(struct hwspinlock *hwlock)
 	return 0;
 }
 
+static inline
+int of_hwspin_lock_get_id_byname(struct device_node *np, const char *name)
+{
+	return 0;
+}
+
 #endif /* !CONFIG_HWSPINLOCK */
 
 /**
-- 
1.7.9.5

[PATCH 2/8] hwspinlock: Add devm_xxx() APIs to request/free hwlock

[Baolin Wang]

This patch introduces some devm_xxx() APIs to help to request or free
the hwlocks, which will help to simplify the cleanup code for drivers
requesting one hwlock, ensuring that the hwlock is automatically freed
whenever the device is unbound.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/hwspinlock/hwspinlock_core.c |  110 ++++++++++++++++++++++++++++++++++
 include/linux/hwspinlock.h           |   22 +++++++
 2 files changed, 132 insertions(+)

diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
index bea3586..d542b6f 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -735,6 +735,116 @@ int hwspin_lock_free(struct hwspinlock *hwlock)
 }
 EXPORT_SYMBOL_GPL(hwspin_lock_free);
 
+static int devm_hwspin_lock_match(struct device *dev, void *res, void *data)
+{
+	struct hwspinlock **hwlock = res;
+
+	if (WARN_ON(!hwlock || !*hwlock))
+		return 0;
+
+	return *hwlock == data;
+}
+
+static void devm_hwspin_lock_release(struct device *dev, void *res)
+{
+	hwspin_lock_free(*(struct hwspinlock **)res);
+}
+
+/**
+ * devm_hwspin_lock_free() - free a specific hwspinlock for a managed device
+ * @dev: the device to free the specific hwspinlock
+ * @hwlock: the specific hwspinlock to free
+ *
+ * This function mark @hwlock as free again.
+ * Should only be called with an @hwlock that was retrieved from
+ * an earlier call to hwspin_lock_request{_specific}.
+ *
+ * Should be called from a process context (might sleep)
+ *
+ * Returns 0 on success, or an appropriate error code on failure
+ */
+int devm_hwspin_lock_free(struct device *dev, struct hwspinlock *hwlock)
+{
+	int ret;
+
+	ret = devres_release(dev, devm_hwspin_lock_release,
+			     devm_hwspin_lock_match, hwlock);
+	WARN_ON(ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_hwspin_lock_free);
+
+/**
+ * devm_hwspin_lock_request() - request an hwspinlock for a managed device
+ * @dev: the device to request an hwspinlock
+ *
+ * This function should be called by users of the hwspinlock device,
+ * in order to dynamically assign them an unused hwspinlock.
+ * Usually the user of this lock will then have to communicate the lock's id
+ * to the remote core before it can be used for synchronization (to get the
+ * id of a given hwlock, use hwspin_lock_get_id()).
+ *
+ * Should be called from a process context (might sleep)
+ *
+ * Returns the address of the assigned hwspinlock, or NULL on error
+ */
+struct hwspinlock *devm_hwspin_lock_request(struct device *dev)
+{
+	struct hwspinlock **ptr, *hwlock;
+
+	ptr = devres_alloc(devm_hwspin_lock_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	hwlock = hwspin_lock_request();
+	if (!IS_ERR(hwlock)) {
+		*ptr = hwlock;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return hwlock;
+}
+EXPORT_SYMBOL_GPL(devm_hwspin_lock_request);
+
+/**
+ * devm_hwspin_lock_request_specific() - request for a specific hwspinlock for
+ *					 a managed device
+ * @dev: the device to request the specific hwspinlock
+ * @id: index of the specific hwspinlock that is requested
+ *
+ * This function should be called by users of the hwspinlock module,
+ * in order to assign them a specific hwspinlock.
+ * Usually early board code will be calling this function in order to
+ * reserve specific hwspinlock ids for predefined purposes.
+ *
+ * Should be called from a process context (might sleep)
+ *
+ * Returns the address of the assigned hwspinlock, or NULL on error
+ */
+struct hwspinlock *devm_hwspin_lock_request_specific(struct device *dev,
+						     unsigned int id)
+{
+	struct hwspinlock **ptr, *hwlock;
+
+	ptr = devres_alloc(devm_hwspin_lock_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	hwlock = hwspin_lock_request_specific(id);
+	if (!IS_ERR(hwlock)) {
+		*ptr = hwlock;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return hwlock;
+}
+EXPORT_SYMBOL_GPL(devm_hwspin_lock_request_specific);
+
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Hardware spinlock interface");
 MODULE_AUTHOR("Ohad Ben-Cohen <ohad@wizery.com>");
diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
index 2b6f389..dfd0593 100644
--- a/include/linux/hwspinlock.h
+++ b/include/linux/hwspinlock.h
@@ -67,6 +67,10 @@ int __hwspin_lock_timeout(struct hwspinlock *, unsigned int, int,
 int __hwspin_trylock(struct hwspinlock *, int, unsigned long *);
 void __hwspin_unlock(struct hwspinlock *, int, unsigned long *);
 int of_hwspin_lock_get_id_byname(struct device_node *np, const char *name);
+int devm_hwspin_lock_free(struct device *dev, struct hwspinlock *hwlock);
+struct hwspinlock *devm_hwspin_lock_request(struct device *dev);
+struct hwspinlock *devm_hwspin_lock_request_specific(struct device *dev,
+						     unsigned int id);
 
 #else /* !CONFIG_HWSPINLOCK */
 
@@ -132,6 +136,24 @@ int of_hwspin_lock_get_id_byname(struct device_node *np, const char *name)
 	return 0;
 }
 
+static inline
+int devm_hwspin_lock_free(struct device *dev, struct hwspinlock *hwlock)
+{
+	return 0;
+}
+
+static inline struct hwspinlock *devm_hwspin_lock_request(struct device *dev)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline
+struct hwspinlock *devm_hwspin_lock_request_specific(struct device *dev,
+						     unsigned int id)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 #endif /* !CONFIG_HWSPINLOCK */
 
 /**
-- 
1.7.9.5

[PATCH 3/8] hwspinlock: Add devm_xxx() APIs to register/unregister one hwlock controller

[Baolin Wang]

This patch introduces devm_hwspin_lock_register() and devm_hwspin_lock_unregister()
interfaces to help to register or unregister one hardware spinlock controller, that
will help to simplify the cleanup code for hwspinlock drivers.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/hwspinlock/hwspinlock_core.c |   82 ++++++++++++++++++++++++++++++++++
 include/linux/hwspinlock.h           |    6 +++
 2 files changed, 88 insertions(+)

diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
index d542b6f..ba27465 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -529,6 +529,88 @@ int hwspin_lock_unregister(struct hwspinlock_device *bank)
 }
 EXPORT_SYMBOL_GPL(hwspin_lock_unregister);
 
+static void devm_hwspin_lock_unreg(struct device *dev, void *res)
+{
+	hwspin_lock_unregister(*(struct hwspinlock_device **)res);
+}
+
+static int devm_hwspin_lock_device_match(struct device *dev, void *res,
+					 void *data)
+{
+	struct hwspinlock_device **bank = res;
+
+	if (WARN_ON(!bank || !*bank))
+		return 0;
+
+	return *bank == data;
+}
+
+/**
+ * devm_hwspin_lock_unregister() - unregister an hw spinlock device for
+ *				   a managed device
+ * @dev: the backing device
+ * @bank: the hwspinlock device, which usually provides numerous hw locks
+ *
+ * This function should be called from the underlying platform-specific
+ * implementation, to unregister an existing (and unused) hwspinlock.
+ *
+ * Should be called from a process context (might sleep)
+ *
+ * Returns 0 on success, or an appropriate error code on failure
+ */
+int devm_hwspin_lock_unregister(struct device *dev,
+				struct hwspinlock_device *bank)
+{
+	int ret;
+
+	ret = devres_release(dev, devm_hwspin_lock_unreg,
+			     devm_hwspin_lock_device_match, bank);
+	WARN_ON(ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_hwspin_lock_unregister);
+
+/**
+ * devm_hwspin_lock_register() - register a new hw spinlock device for
+ *				 a managed device
+ * @dev: the backing device
+ * @bank: the hwspinlock device, which usually provides numerous hw locks
+ * @ops: hwspinlock handlers for this device
+ * @base_id: id of the first hardware spinlock in this bank
+ * @num_locks: number of hwspinlocks provided by this device
+ *
+ * This function should be called from the underlying platform-specific
+ * implementation, to register a new hwspinlock device instance.
+ *
+ * Should be called from a process context (might sleep)
+ *
+ * Returns 0 on success, or an appropriate error code on failure
+ */
+int devm_hwspin_lock_register(struct device *dev,
+			      struct hwspinlock_device *bank,
+			      const struct hwspinlock_ops *ops,
+			      int base_id, int num_locks)
+{
+	struct hwspinlock_device **ptr;
+	int ret;
+
+	ptr = devres_alloc(devm_hwspin_lock_unreg, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	ret = hwspin_lock_register(bank, dev, ops, base_id, num_locks);
+	if (!ret) {
+		*ptr = bank;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_hwspin_lock_register);
+
 /**
  * __hwspin_lock_request() - tag an hwspinlock as used and power it up
  *
diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
index dfd0593..94064cc 100644
--- a/include/linux/hwspinlock.h
+++ b/include/linux/hwspinlock.h
@@ -71,6 +71,12 @@ int __hwspin_lock_timeout(struct hwspinlock *, unsigned int, int,
 struct hwspinlock *devm_hwspin_lock_request(struct device *dev);
 struct hwspinlock *devm_hwspin_lock_request_specific(struct device *dev,
 						     unsigned int id);
+int devm_hwspin_lock_unregister(struct device *dev,
+				struct hwspinlock_device *bank);
+int devm_hwspin_lock_register(struct device *dev,
+			      struct hwspinlock_device *bank,
+			      const struct hwspinlock_ops *ops,
+			      int base_id, int num_locks);
 
 #else /* !CONFIG_HWSPINLOCK */
 
-- 
1.7.9.5

[PATCH 4/8] hwspinlock: Remove redundant config

[Baolin Wang]

The hardware core can not be built as a module, so remove the redundant
CONFIG_HWSPINLOCK_MODULE config.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 include/linux/hwspinlock.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
index 94064cc..0afe693 100644
--- a/include/linux/hwspinlock.h
+++ b/include/linux/hwspinlock.h
@@ -52,7 +52,7 @@ struct hwspinlock_pdata {
 	int base_id;
 };
 
-#if defined(CONFIG_HWSPINLOCK) || defined(CONFIG_HWSPINLOCK_MODULE)
+#ifdef CONFIG_HWSPINLOCK
 
 int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev,
 		const struct hwspinlock_ops *ops, int base_id, int num_locks);
-- 
1.7.9.5

[PATCH 5/8] hwspinlock: Fix one comment mistake

[Baolin Wang]

Fix one comment mistake with correct function names.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/hwspinlock/hwspinlock_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
index ba27465..e16d648 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -767,7 +767,7 @@ struct hwspinlock *hwspin_lock_request_specific(unsigned int id)
  *
  * This function mark @hwlock as free again.
  * Should only be called with an @hwlock that was retrieved from
- * an earlier call to omap_hwspin_lock_request{_specific}.
+ * an earlier call to hwspin_lock_request{_specific}.
  *
  * Should be called from a process context (might sleep)
  *
-- 
1.7.9.5

[PATCH 6/8] hwspinlock: sprd: Use devm_hwspin_lock_register()

[Baolin Wang]

Use devm_hwspin_lock_register() to register the hwlock controller to
avoid unregistering the device explicitly.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/hwspinlock/sprd_hwspinlock.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hwspinlock/sprd_hwspinlock.c b/drivers/hwspinlock/sprd_hwspinlock.c
index dc42bf5..ba5daf6 100644
--- a/drivers/hwspinlock/sprd_hwspinlock.c
+++ b/drivers/hwspinlock/sprd_hwspinlock.c
@@ -120,8 +120,9 @@ static int sprd_hwspinlock_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, sprd_hwlock);
 	pm_runtime_enable(&pdev->dev);
 
-	ret = hwspin_lock_register(&sprd_hwlock->bank, &pdev->dev,
-				   &sprd_hwspinlock_ops, 0, SPRD_HWLOCKS_NUM);
+	ret = devm_hwspin_lock_register(&pdev->dev, &sprd_hwlock->bank,
+					&sprd_hwspinlock_ops, 0,
+					SPRD_HWLOCKS_NUM);
 	if (ret) {
 		pm_runtime_disable(&pdev->dev);
 		clk_disable_unprepare(sprd_hwlock->clk);
@@ -135,7 +136,6 @@ static int sprd_hwspinlock_remove(struct platform_device *pdev)
 {
 	struct sprd_hwspinlock_dev *sprd_hwlock = platform_get_drvdata(pdev);
 
-	hwspin_lock_unregister(&sprd_hwlock->bank);
 	pm_runtime_disable(&pdev->dev);
 	clk_disable_unprepare(sprd_hwlock->clk);
 	return 0;
-- 
1.7.9.5

[PATCH 7/8] spi: sprd: Replace of_hwspin_lock_get_id() with of_hwspin_lock_get_id_byname()

[Baolin Wang]

Now the hwlock core has supplied new function to get a specific hwlock id
by one hwlock name, which is more clear for users. So change to use
of_hwspin_lock_get_id_byname().

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/spi/spi-sprd-adi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
index 6fc50cf..d8d5e2f 100644
--- a/drivers/spi/spi-sprd-adi.c
+++ b/drivers/spi/spi-sprd-adi.c
@@ -459,7 +459,7 @@ static int sprd_adi_probe(struct platform_device *pdev)
 	sadi->slave_pbase = res->start + ADI_SLAVE_OFFSET;
 	sadi->ctlr = ctlr;
 	sadi->dev = &pdev->dev;
-	ret = of_hwspin_lock_get_id(np, 0);
+	ret = of_hwspin_lock_get_id_byname(np, "adi");
 	if (ret < 0) {
 		dev_err(&pdev->dev, "can not get the hardware spinlock\n");
 		goto put_ctlr;
-- 
1.7.9.5

[PATCH 8/8] spi: sprd: Change to use devm_hwspin_lock_request_specific()

[Baolin Wang]

Change to use devm_hwspin_lock_request_specific() instead of
freeing the hwlock explicitly when unbound the device.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/spi/spi-sprd-adi.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
index d8d5e2f..6fb5c0c 100644
--- a/drivers/spi/spi-sprd-adi.c
+++ b/drivers/spi/spi-sprd-adi.c
@@ -465,7 +465,7 @@ static int sprd_adi_probe(struct platform_device *pdev)
 		goto put_ctlr;
 	}
 
-	sadi->hwlock = hwspin_lock_request_specific(ret);
+	sadi->hwlock = devm_hwspin_lock_request_specific(&pdev->dev, ret);
 	if (!sadi->hwlock) {
 		ret = -ENXIO;
 		goto put_ctlr;
@@ -483,7 +483,7 @@ static int sprd_adi_probe(struct platform_device *pdev)
 	ret = devm_spi_register_controller(&pdev->dev, ctlr);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register SPI controller\n");
-		goto free_hwlock;
+		goto put_ctlr;
 	}
 
 	sadi->restart_handler.notifier_call = sprd_adi_restart_handler;
@@ -491,13 +491,11 @@ static int sprd_adi_probe(struct platform_device *pdev)
 	ret = register_restart_handler(&sadi->restart_handler);
 	if (ret) {
 		dev_err(&pdev->dev, "can not register restart handler\n");
-		goto free_hwlock;
+		goto put_ctlr;
 	}
 
 	return 0;
 
-free_hwlock:
-	hwspin_lock_free(sadi->hwlock);
 put_ctlr:
 	spi_controller_put(ctlr);
 	return ret;
@@ -509,7 +507,6 @@ static int sprd_adi_remove(struct platform_device *pdev)
 	struct sprd_adi *sadi = spi_controller_get_devdata(ctlr);
 
 	unregister_restart_handler(&sadi->restart_handler);
-	hwspin_lock_free(sadi->hwlock);
 	return 0;
 }
 
-- 
1.7.9.5

Re: [PATCH 7/8] spi: sprd: Replace of_hwspin_lock_get_id() with of_hwspin_lock_get_id_byname()

[Mark Brown]

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

On Fri, Jun 22, 2018 at 04:09:04PM +0800, Baolin Wang wrote:
> Now the hwlock core has supplied new function to get a specific hwlock id
> by one hwlock name, which is more clear for users. So change to use
> of_hwspin_lock_get_id_byname().

Acked-by: Mark Brown <broonie@kernel.org>

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

Re: [PATCH 8/8] spi: sprd: Change to use devm_hwspin_lock_request_specific()

[Mark Brown]

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

On Fri, Jun 22, 2018 at 04:09:05PM +0800, Baolin Wang wrote:
> Change to use devm_hwspin_lock_request_specific() instead of
> freeing the hwlock explicitly when unbound the device.

Acked-by: Mark Brown <broonie@kernel.org>

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

Re: [PATCH 6/8] hwspinlock: sprd: Use devm_hwspin_lock_register()

[Bjorn Andersson]

On Fri 22 Jun 01:09 PDT 2018, Baolin Wang wrote:

> Use devm_hwspin_lock_register() to register the hwlock controller to
> avoid unregistering the device explicitly.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
>  drivers/hwspinlock/sprd_hwspinlock.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwspinlock/sprd_hwspinlock.c b/drivers/hwspinlock/sprd_hwspinlock.c
> index dc42bf5..ba5daf6 100644
> --- a/drivers/hwspinlock/sprd_hwspinlock.c
> +++ b/drivers/hwspinlock/sprd_hwspinlock.c
> @@ -120,8 +120,9 @@ static int sprd_hwspinlock_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, sprd_hwlock);
>  	pm_runtime_enable(&pdev->dev);
>  
> -	ret = hwspin_lock_register(&sprd_hwlock->bank, &pdev->dev,
> -				   &sprd_hwspinlock_ops, 0, SPRD_HWLOCKS_NUM);
> +	ret = devm_hwspin_lock_register(&pdev->dev, &sprd_hwlock->bank,
> +					&sprd_hwspinlock_ops, 0,
> +					SPRD_HWLOCKS_NUM);
>  	if (ret) {
>  		pm_runtime_disable(&pdev->dev);
>  		clk_disable_unprepare(sprd_hwlock->clk);
> @@ -135,7 +136,6 @@ static int sprd_hwspinlock_remove(struct platform_device *pdev)
>  {
>  	struct sprd_hwspinlock_dev *sprd_hwlock = platform_get_drvdata(pdev);
>  
> -	hwspin_lock_unregister(&sprd_hwlock->bank);

Until the hwspinlock is unregistered it's possible for clients to issue
operations on it, so disabling power and clocks before that seems bad.

Regards,
Bjorn

>  	pm_runtime_disable(&pdev->dev);
>  	clk_disable_unprepare(sprd_hwlock->clk);
>  	return 0;
> -- 
> 1.7.9.5
> 

Re: [PATCH 1/8] hwspinlock: Add one new API to support getting a specific hwlock by the name

[Bjorn Andersson]

On Fri 22 Jun 01:08 PDT 2018, Baolin Wang wrote:

> The hardware spinlock binding already supplied the 'hwlock-names' property
> to match and get a specific hwlock, but did not supply one API for users
> to get a specific hwlock by the hwlock name. So this patch introduces one
> API to support this requirement.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>

Thanks Baolin,

Picked up all but patch 6 from the series; including 7 and 8 with Mark's
ack.

Regards,
Bjorn

> ---
>  drivers/hwspinlock/hwspinlock_core.c |   29 +++++++++++++++++++++++++++++
>  include/linux/hwspinlock.h           |    7 +++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
> index d16e6a3..bea3586 100644
> --- a/drivers/hwspinlock/hwspinlock_core.c
> +++ b/drivers/hwspinlock/hwspinlock_core.c
> @@ -367,6 +367,35 @@ int of_hwspin_lock_get_id(struct device_node *np, int index)
>  }
>  EXPORT_SYMBOL_GPL(of_hwspin_lock_get_id);
>  
> +/**
> + * of_hwspin_lock_get_id_byname() - get lock id for an specified hwlock name
> + * @np: device node from which to request the specific hwlock
> + * @name: hwlock name
> + *
> + * This function provides a means for DT users of the hwspinlock module to
> + * get the global lock id of a specific hwspinlock using the specified name of
> + * the hwspinlock device, so that it can be requested using the normal
> + * hwspin_lock_request_specific() API.
> + *
> + * Returns the global lock id number on success, -EPROBE_DEFER if the hwspinlock
> + * device is not yet registered, -EINVAL on invalid args specifier value or an
> + * appropriate error as returned from the OF parsing of the DT client node.
> + */
> +int of_hwspin_lock_get_id_byname(struct device_node *np, const char *name)
> +{
> +	int index;
> +
> +	if (!name)
> +		return -EINVAL;
> +
> +	index = of_property_match_string(np, "hwlock-names", name);
> +	if (index < 0)
> +		return index;
> +
> +	return of_hwspin_lock_get_id(np, index);
> +}
> +EXPORT_SYMBOL_GPL(of_hwspin_lock_get_id_byname);
> +
>  static int hwspin_lock_register_single(struct hwspinlock *hwlock, int id)
>  {
>  	struct hwspinlock *tmp;
> diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
> index 57537e6..2b6f389 100644
> --- a/include/linux/hwspinlock.h
> +++ b/include/linux/hwspinlock.h
> @@ -66,6 +66,7 @@ int __hwspin_lock_timeout(struct hwspinlock *, unsigned int, int,
>  							unsigned long *);
>  int __hwspin_trylock(struct hwspinlock *, int, unsigned long *);
>  void __hwspin_unlock(struct hwspinlock *, int, unsigned long *);
> +int of_hwspin_lock_get_id_byname(struct device_node *np, const char *name);
>  
>  #else /* !CONFIG_HWSPINLOCK */
>  
> @@ -125,6 +126,12 @@ static inline int hwspin_lock_get_id(struct hwspinlock *hwlock)
>  	return 0;
>  }
>  
> +static inline
> +int of_hwspin_lock_get_id_byname(struct device_node *np, const char *name)
> +{
> +	return 0;
> +}
> +
>  #endif /* !CONFIG_HWSPINLOCK */
>  
>  /**
> -- 
> 1.7.9.5
> 

Re: [PATCH 1/8] hwspinlock: Add one new API to support getting a specific hwlock by the name

[Bjorn Andersson]

On Fri 22 Jun 01:08 PDT 2018, Baolin Wang wrote:

> The hardware spinlock binding already supplied the 'hwlock-names' property
> to match and get a specific hwlock, but did not supply one API for users
> to get a specific hwlock by the hwlock name. So this patch introduces one
> API to support this requirement.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>

Thanks Baolin,

Picked up all but patch 6 from the series; including 7 and 8 with Mark's
ack.

Regards,
Bjorn

> ---
>  drivers/hwspinlock/hwspinlock_core.c |   29 +++++++++++++++++++++++++++++
>  include/linux/hwspinlock.h           |    7 +++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
> index d16e6a3..bea3586 100644
> --- a/drivers/hwspinlock/hwspinlock_core.c
> +++ b/drivers/hwspinlock/hwspinlock_core.c
> @@ -367,6 +367,35 @@ int of_hwspin_lock_get_id(struct device_node *np, int index)
>  }
>  EXPORT_SYMBOL_GPL(of_hwspin_lock_get_id);
>  
> +/**
> + * of_hwspin_lock_get_id_byname() - get lock id for an specified hwlock name
> + * @np: device node from which to request the specific hwlock
> + * @name: hwlock name
> + *
> + * This function provides a means for DT users of the hwspinlock module to
> + * get the global lock id of a specific hwspinlock using the specified name of
> + * the hwspinlock device, so that it can be requested using the normal
> + * hwspin_lock_request_specific() API.
> + *
> + * Returns the global lock id number on success, -EPROBE_DEFER if the hwspinlock
> + * device is not yet registered, -EINVAL on invalid args specifier value or an
> + * appropriate error as returned from the OF parsing of the DT client node.
> + */
> +int of_hwspin_lock_get_id_byname(struct device_node *np, const char *name)
> +{
> +	int index;
> +
> +	if (!name)
> +		return -EINVAL;
> +
> +	index = of_property_match_string(np, "hwlock-names", name);
> +	if (index < 0)
> +		return index;
> +
> +	return of_hwspin_lock_get_id(np, index);
> +}
> +EXPORT_SYMBOL_GPL(of_hwspin_lock_get_id_byname);
> +
>  static int hwspin_lock_register_single(struct hwspinlock *hwlock, int id)
>  {
>  	struct hwspinlock *tmp;
> diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
> index 57537e6..2b6f389 100644
> --- a/include/linux/hwspinlock.h
> +++ b/include/linux/hwspinlock.h
> @@ -66,6 +66,7 @@ int __hwspin_lock_timeout(struct hwspinlock *, unsigned int, int,
>  							unsigned long *);
>  int __hwspin_trylock(struct hwspinlock *, int, unsigned long *);
>  void __hwspin_unlock(struct hwspinlock *, int, unsigned long *);
> +int of_hwspin_lock_get_id_byname(struct device_node *np, const char *name);
>  
>  #else /* !CONFIG_HWSPINLOCK */
>  
> @@ -125,6 +126,12 @@ static inline int hwspin_lock_get_id(struct hwspinlock *hwlock)
>  	return 0;
>  }
>  
> +static inline
> +int of_hwspin_lock_get_id_byname(struct device_node *np, const char *name)
> +{
> +	return 0;
> +}
> +
>  #endif /* !CONFIG_HWSPINLOCK */
>  
>  /**
> -- 
> 1.7.9.5
> 

Re: [PATCH 6/8] hwspinlock: sprd: Use devm_hwspin_lock_register()

[Baolin Wang]

On 27 June 2018 at 04:51, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
> On Fri 22 Jun 01:09 PDT 2018, Baolin Wang wrote:
>
>> Use devm_hwspin_lock_register() to register the hwlock controller to
>> avoid unregistering the device explicitly.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> ---
>>  drivers/hwspinlock/sprd_hwspinlock.c |    6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwspinlock/sprd_hwspinlock.c b/drivers/hwspinlock/sprd_hwspinlock.c
>> index dc42bf5..ba5daf6 100644
>> --- a/drivers/hwspinlock/sprd_hwspinlock.c
>> +++ b/drivers/hwspinlock/sprd_hwspinlock.c
>> @@ -120,8 +120,9 @@ static int sprd_hwspinlock_probe(struct platform_device *pdev)
>>       platform_set_drvdata(pdev, sprd_hwlock);
>>       pm_runtime_enable(&pdev->dev);
>>
>> -     ret = hwspin_lock_register(&sprd_hwlock->bank, &pdev->dev,
>> -                                &sprd_hwspinlock_ops, 0, SPRD_HWLOCKS_NUM);
>> +     ret = devm_hwspin_lock_register(&pdev->dev, &sprd_hwlock->bank,
>> +                                     &sprd_hwspinlock_ops, 0,
>> +                                     SPRD_HWLOCKS_NUM);
>>       if (ret) {
>>               pm_runtime_disable(&pdev->dev);
>>               clk_disable_unprepare(sprd_hwlock->clk);
>> @@ -135,7 +136,6 @@ static int sprd_hwspinlock_remove(struct platform_device *pdev)
>>  {
>>       struct sprd_hwspinlock_dev *sprd_hwlock = platform_get_drvdata(pdev);
>>
>> -     hwspin_lock_unregister(&sprd_hwlock->bank);
>
> Until the hwspinlock is unregistered it's possible for clients to issue
> operations on it, so disabling power and clocks before that seems bad.

Yes, you are right. So may be register some additional actions with devm
core to call the clock disable as appropriate. Thanks.

-- 
Baolin.wang
Best Regards

Re: [PATCH 1/8] hwspinlock: Add one new API to support getting a specific hwlock by the name

[Baolin Wang]

On 27 June 2018 at 04:54, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
> On Fri 22 Jun 01:08 PDT 2018, Baolin Wang wrote:
>
>> The hardware spinlock binding already supplied the 'hwlock-names' property
>> to match and get a specific hwlock, but did not supply one API for users
>> to get a specific hwlock by the hwlock name. So this patch introduces one
>> API to support this requirement.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>
> Thanks Baolin,
>
> Picked up all but patch 6 from the series; including 7 and 8 with Mark's
> ack.

OK. Thanks Bjorn and Mark.

-- 
Baolin.wang
Best Regards