[alsa-devel] [PATCH 0/4] slimbus: minor fixes.
![](https://secure.gravatar.com/avatar/b1a3065ae9c42727d7ce4fa7689b8c07.jpg?s=120&d=mm&r=g)
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(-)
![](https://secure.gravatar.com/avatar/b1a3065ae9c42727d7ce4fa7689b8c07.jpg?s=120&d=mm&r=g)
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++; }
![](https://secure.gravatar.com/avatar/b1a3065ae9c42727d7ce4fa7689b8c07.jpg?s=120&d=mm&r=g)
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;
![](https://secure.gravatar.com/avatar/b1a3065ae9c42727d7ce4fa7689b8c07.jpg?s=120&d=mm&r=g)
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
![](https://secure.gravatar.com/avatar/b1a3065ae9c42727d7ce4fa7689b8c07.jpg?s=120&d=mm&r=g)
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);
![](https://secure.gravatar.com/avatar/cbd18395260b6be2575187286a262f9a.jpg?s=120&d=mm&r=g)
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
participants (2)
-
Greg KH
-
Srinivas Kandagatla