[PATCH 0/4] slimbus: minor fixes.

[PATCH 0/4] slimbus: minor fixes.

[Srinivas Kandagatla]

Here are some minor fixes to slimbus core for issues found while testing wcd9335.
First one is make the slimbus core match full device id,
second one is to avoid using big lock in status callback function,
third one is to use status function rather than directly invoking the status ops.
finally last one is to fix missing spinlock init.

Thanks,
srini


Srinivas Kandagatla (4):
  slimbus: core: match full device id
  slimbus: core: do not call device_status under a lock
  slimbus: core: use slim_device_update_status() to update status
  slimbus: core: add missing spin_lock_init on txn_lock

 drivers/slimbus/core.c | 45 +++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

-- 
2.20.1

[PATCH 1/4] slimbus: core: match full device id

[Srinivas Kandagatla]

match full slim device id instead of just product and manufacture code,
this will allow drivers to be much more specific to that device.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/slimbus/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/slimbus/core.c b/drivers/slimbus/core.c
index 55eda5863a6b..9e4f3b21ba8c 100644
--- a/drivers/slimbus/core.c
+++ b/drivers/slimbus/core.c
@@ -21,7 +21,9 @@ static const struct slim_device_id *slim_match(const struct slim_device_id *id,
 {
 	while (id->manf_id != 0 || id->prod_code != 0) {
 		if (id->manf_id == sbdev->e_addr.manf_id &&
-		    id->prod_code == sbdev->e_addr.prod_code)
+		    id->prod_code == sbdev->e_addr.prod_code &&
+		    id->dev_index == sbdev->e_addr.dev_index &&
+		    id->instance == sbdev->e_addr.instance)
 			return id;
 		id++;
 	}
-- 
2.20.1

[PATCH 1/4] slimbus: core: match full device id

[Srinivas Kandagatla]

match full slim device id instead of just product and manufacture code,
this will allow drivers to be much more specific to that device.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/slimbus/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/slimbus/core.c b/drivers/slimbus/core.c
index 55eda5863a6b..9e4f3b21ba8c 100644
--- a/drivers/slimbus/core.c
+++ b/drivers/slimbus/core.c
@@ -21,7 +21,9 @@ static const struct slim_device_id *slim_match(const struct slim_device_id *id,
 {
 	while (id->manf_id != 0 || id->prod_code != 0) {
 		if (id->manf_id == sbdev->e_addr.manf_id &&
-		    id->prod_code == sbdev->e_addr.prod_code)
+		    id->prod_code == sbdev->e_addr.prod_code &&
+		    id->dev_index == sbdev->e_addr.dev_index &&
+		    id->instance == sbdev->e_addr.instance)
 			return id;
 		id++;
 	}
-- 
2.20.1

[PATCH 2/4] slimbus: core: do not call device_status under a lock

[Srinivas Kandagatla]

Calling device_status callback under a lock would prevent drivers
to do any slimbus trasactions which would invoke this lock like
get_laddr(). Remove this unnecessary lock!

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/slimbus/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/slimbus/core.c b/drivers/slimbus/core.c
index 9e4f3b21ba8c..1ba08b5f2e78 100644
--- a/drivers/slimbus/core.c
+++ b/drivers/slimbus/core.c
@@ -466,6 +466,7 @@ static int slim_device_alloc_laddr(struct slim_device *sbdev,
 
 	sbdev->laddr = laddr;
 	sbdev->is_laddr_valid = true;
+	mutex_unlock(&ctrl->lock);
 
 	slim_device_update_status(sbdev, SLIM_DEVICE_STATUS_UP);
 
@@ -473,6 +474,8 @@ static int slim_device_alloc_laddr(struct slim_device *sbdev,
 		laddr, sbdev->e_addr.manf_id, sbdev->e_addr.prod_code,
 		sbdev->e_addr.dev_index, sbdev->e_addr.instance);
 
+	return 0;
+
 err:
 	mutex_unlock(&ctrl->lock);
 	return ret;
-- 
2.20.1

[PATCH 3/4] slimbus: core: use slim_device_update_status() to update status

[Srinivas Kandagatla]

use slim_device_update_status() instead of directly calling
the device_status() callback. Move slim_device_update_status() before
probe to avoid forward declaration too.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/slimbus/core.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/slimbus/core.c b/drivers/slimbus/core.c
index 1ba08b5f2e78..8ffdc9876223 100644
--- a/drivers/slimbus/core.c
+++ b/drivers/slimbus/core.c
@@ -42,6 +42,23 @@ static int slim_device_match(struct device *dev, struct device_driver *drv)
 	return !!slim_match(sbdrv->id_table, sbdev);
 }
 
+static void slim_device_update_status(struct slim_device *sbdev,
+				      enum slim_device_status status)
+{
+	struct slim_driver *sbdrv;
+
+	if (sbdev->status == status)
+		return;
+
+	sbdev->status = status;
+	if (!sbdev->dev.driver)
+		return;
+
+	sbdrv = to_slim_driver(sbdev->dev.driver);
+	if (sbdrv->device_status)
+		sbdrv->device_status(sbdev, sbdev->status);
+}
+
 static int slim_device_probe(struct device *dev)
 {
 	struct slim_device	*sbdev = to_slim_device(dev);
@@ -55,8 +72,7 @@ static int slim_device_probe(struct device *dev)
 	/* try getting the logical address after probe */
 	ret = slim_get_logical_addr(sbdev);
 	if (!ret) {
-		if (sbdrv->device_status)
-			sbdrv->device_status(sbdev, sbdev->status);
+		slim_device_update_status(sbdev, SLIM_DEVICE_STATUS_UP);
 	} else {
 		dev_err(&sbdev->dev, "Failed to get logical address\n");
 		ret = -EPROBE_DEFER;
@@ -297,23 +313,6 @@ int slim_unregister_controller(struct slim_controller *ctrl)
 }
 EXPORT_SYMBOL_GPL(slim_unregister_controller);
 
-static void slim_device_update_status(struct slim_device *sbdev,
-				      enum slim_device_status status)
-{
-	struct slim_driver *sbdrv;
-
-	if (sbdev->status == status)
-		return;
-
-	sbdev->status = status;
-	if (!sbdev->dev.driver)
-		return;
-
-	sbdrv = to_slim_driver(sbdev->dev.driver);
-	if (sbdrv->device_status)
-		sbdrv->device_status(sbdev, sbdev->status);
-}
-
 /**
  * slim_report_absent() - Controller calls this function when a device
  *	reports absent, OR when the device cannot be communicated with
-- 
2.20.1

[PATCH 4/4] slimbus: core: add missing spin_lock_init on txn_lock

[Srinivas Kandagatla]

Add missing spin lock intialization this also fixes the spinlock bad magic
warning.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/slimbus/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/slimbus/core.c b/drivers/slimbus/core.c
index 8ffdc9876223..b2f07d2043eb 100644
--- a/drivers/slimbus/core.c
+++ b/drivers/slimbus/core.c
@@ -274,6 +274,7 @@ int slim_register_controller(struct slim_controller *ctrl)
 	mutex_init(&ctrl->lock);
 	mutex_init(&ctrl->sched.m_reconf);
 	init_completion(&ctrl->sched.pause_comp);
+	spin_lock_init(&ctrl->txn_lock);
 
 	dev_dbg(ctrl->dev, "Bus [%s] registered:dev:%p\n",
 		ctrl->name, ctrl->dev);
-- 
2.20.1

Re: [PATCH 0/4] slimbus: minor fixes.

[Greg KH]

On Mon, Jan 14, 2019 at 01:44:45PM +0000, Srinivas Kandagatla wrote:
> Here are some minor fixes to slimbus core for issues found while testing wcd9335.
> First one is make the slimbus core match full device id,
> second one is to avoid using big lock in status callback function,
> third one is to use status function rather than directly invoking the status ops.
> finally last one is to fix missing spinlock init.

Should all of these go into 5.0-final?  backported to any older releases
as well?  Or can we wait until 5.1-rc1?

thanks,

greg k-h

Re: [PATCH 0/4] slimbus: minor fixes.

[Srinivas Kandagatla]

On 14/01/2019 14:53, Greg KH wrote:
> Should all of these go into 5.0-final?  backported to any older releases
> as well?  Or can we wait until 5.1-rc1?

We can wait until 5.1-rc1

thanks,
srini
> 
> thanks,