[alsa-devel] [PATCH 0/7] slimbus: fix module loading
From: Srinivas Kandagatla srinivas.kandagatla@linaro.org
Hi Greg,
Here are some of the fixes for issues found while testing module loading on DB820c debian setup.
First 3 core fixes are to do with generating uevents, calling status callback in probe sequence if the device is up and one fix is able to match device id from device tree compatible strings. Second 4 ngd patches fixes few issues firstly with registration, secondly checking the validity of logical address assigned by remote. Lastly there are few typo fixes.
Thanks, Srini
Srinivas Kandagatla (7): slimbus: core: add support to uevent slimbus: core: update device status in probe slimbus: core: match device tree based devices correctly slimbus: ngd: validate logical address assigned by remote silmbus: ngd: register controller after power up. slimbus: ngd: return proper error code instead of zero slimbus: ngd: register ngd driver only once.
drivers/slimbus/core.c | 37 +++++++++++++++++++++++++++++++++++-- drivers/slimbus/qcom-ngd-ctrl.c | 28 +++++++++++++++++----------- 2 files changed, 52 insertions(+), 13 deletions(-)
From: Srinivas Kandagatla srinivas.kandagatla@linaro.org
This patch adds support to uevent to help automatic module loading.
Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/core.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/slimbus/core.c b/drivers/slimbus/core.c index 95b00d2..31f2910 100644 --- a/drivers/slimbus/core.c +++ b/drivers/slimbus/core.c @@ -9,6 +9,7 @@ #include <linux/init.h> #include <linux/idr.h> #include <linux/of.h> +#include <linux/of_device.h> #include <linux/pm_runtime.h> #include <linux/slimbus.h> #include "slimbus.h" @@ -57,11 +58,24 @@ static int slim_device_remove(struct device *dev) return 0; }
+static int slim_device_uevent(struct device *dev, struct kobj_uevent_env *env) +{ + struct slim_device *sbdev = to_slim_device(dev); + int ret; + + ret = of_device_uevent_modalias(dev, env); + if (ret != -ENODEV) + return ret; + + return add_uevent_var(env, "MODALIAS=slim:%s", dev_name(&sbdev->dev)); +} + struct bus_type slimbus_bus = { .name = "slimbus", .match = slim_device_match, .probe = slim_device_probe, .remove = slim_device_remove, + .uevent = slim_device_uevent, }; EXPORT_SYMBOL_GPL(slimbus_bus);
From: Srinivas Kandagatla srinivas.kandagatla@linaro.org
device status update can be racy with probe in some cases, so make sure it take lock during the probe. Also after probe the device is expected to be ready for communications, so make sure that a logical address can be assigned to it after probe. If it fails to do so then probe defer such instances.
Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/core.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/slimbus/core.c b/drivers/slimbus/core.c index 31f2910..262591f 100644 --- a/drivers/slimbus/core.c +++ b/drivers/slimbus/core.c @@ -40,8 +40,23 @@ static int slim_device_probe(struct device *dev) { struct slim_device *sbdev = to_slim_device(dev); struct slim_driver *sbdrv = to_slim_driver(dev->driver); + int ret; + + ret = sbdrv->probe(sbdev); + if (ret) + return ret;
- return sbdrv->probe(sbdev); + /* 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); + } else { + dev_err(&sbdev->dev, "Failed to get logical address\n"); + ret = -EPROBE_DEFER; + } + + return ret; }
static int slim_device_remove(struct device *dev)
From: Srinivas Kandagatla srinivas.kandagatla@linaro.org
device_id for device tree based devices come from dt compatible string, such drivers need not provide non dt style device id table.
Match those device using compatible strings.
Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/core.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/slimbus/core.c b/drivers/slimbus/core.c index 262591f..55eda58 100644 --- a/drivers/slimbus/core.c +++ b/drivers/slimbus/core.c @@ -33,6 +33,10 @@ static int slim_device_match(struct device *dev, struct device_driver *drv) struct slim_device *sbdev = to_slim_device(dev); struct slim_driver *sbdrv = to_slim_driver(drv);
+ /* Attempt an OF style match first */ + if (of_driver_match_device(dev, drv)) + return 1; + return !!slim_match(sbdrv->id_table, sbdev); }
@@ -106,7 +110,7 @@ EXPORT_SYMBOL_GPL(slimbus_bus); int __slim_driver_register(struct slim_driver *drv, struct module *owner) { /* ID table and probe are mandatory */ - if (!drv->id_table || !drv->probe) + if (!(drv->driver.of_match_table || drv->id_table) || !drv->probe) return -EINVAL;
drv->driver.bus = &slimbus_bus;
From: Srinivas Kandagatla srinivas.kandagatla@linaro.org
Validate logical address assigned by remote, in failure cases this value is all zeors.
Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/qcom-ngd-ctrl.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c index 8be4d67..b9f2e3f 100644 --- a/drivers/slimbus/qcom-ngd-ctrl.c +++ b/drivers/slimbus/qcom-ngd-ctrl.c @@ -1004,6 +1004,7 @@ static int qcom_slim_ngd_get_laddr(struct slim_controller *ctrl, struct slim_eaddr *ea, u8 *laddr) { struct slim_val_inf msg = {0}; + u8 failed_ea[6] = {0, 0, 0, 0, 0, 0}; struct slim_msg_txn txn; u8 wbuf[10] = {0}; u8 rbuf[10] = {0}; @@ -1034,6 +1035,9 @@ static int qcom_slim_ngd_get_laddr(struct slim_controller *ctrl, return ret; }
+ if (!memcmp(rbuf, failed_ea, 6)) + return -ENXIO; + *laddr = rbuf[6];
return ret;
From: Srinivas Kandagatla srinivas.kandagatla@linaro.org
Register slimbus controller only after finishing powerup sequnce so that we do not endup in situation where core starts sending transactions before the controller is ready.
Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/qcom-ngd-ctrl.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c index b9f2e3f..0241373 100644 --- a/drivers/slimbus/qcom-ngd-ctrl.c +++ b/drivers/slimbus/qcom-ngd-ctrl.c @@ -1238,8 +1238,17 @@ static int qcom_slim_ngd_enable(struct qcom_slim_ngd_ctrl *ctrl, bool enable) pm_runtime_resume(ctrl->dev); pm_runtime_mark_last_busy(ctrl->dev); pm_runtime_put(ctrl->dev); + + ret = slim_register_controller(&ctrl->ctrl); + if (ret) { + dev_err(ctrl->dev, "error adding slim controller\n"); + return ret; + } + + dev_info(ctrl->dev, "SLIM controller Registered\n"); } else { qcom_slim_qmi_exit(ctrl); + slim_unregister_controller(&ctrl->ctrl); }
return 0; @@ -1361,11 +1370,6 @@ static int qcom_slim_ngd_probe(struct platform_device *pdev) int ret;
ctrl->ctrl.dev = dev; - ret = slim_register_controller(&ctrl->ctrl); - if (ret) { - dev_err(dev, "error adding slim controller\n"); - return ret; - }
pm_runtime_use_autosuspend(dev); pm_runtime_set_autosuspend_delay(dev, QCOM_SLIM_NGD_AUTOSUSPEND); @@ -1375,7 +1379,7 @@ static int qcom_slim_ngd_probe(struct platform_device *pdev) ret = qcom_slim_ngd_qmi_svc_event_init(ctrl); if (ret) { dev_err(&pdev->dev, "QMI service registration failed:%d", ret); - goto err; + return ret; }
INIT_WORK(&ctrl->m_work, qcom_slim_ngd_master_worker); @@ -1387,8 +1391,6 @@ static int qcom_slim_ngd_probe(struct platform_device *pdev) }
return 0; -err: - slim_unregister_controller(&ctrl->ctrl); wq_err: qcom_slim_ngd_qmi_svc_event_deinit(&ctrl->qmi); if (ctrl->mwq) @@ -1460,7 +1462,7 @@ static int qcom_slim_ngd_remove(struct platform_device *pdev) struct qcom_slim_ngd_ctrl *ctrl = platform_get_drvdata(pdev);
pm_runtime_disable(&pdev->dev); - slim_unregister_controller(&ctrl->ctrl); + qcom_slim_ngd_enable(ctrl, false); qcom_slim_ngd_exit_dma(ctrl); qcom_slim_ngd_qmi_svc_event_deinit(&ctrl->qmi); if (ctrl->mwq)
From: Srinivas Kandagatla srinivas.kandagatla@linaro.org
It looks like there is a typo in probe return. Fix it.
Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/qcom-ngd-ctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c index 0241373..f48a06e 100644 --- a/drivers/slimbus/qcom-ngd-ctrl.c +++ b/drivers/slimbus/qcom-ngd-ctrl.c @@ -1396,7 +1396,7 @@ static int qcom_slim_ngd_probe(struct platform_device *pdev) if (ctrl->mwq) destroy_workqueue(ctrl->mwq);
- return 0; + return ret; }
static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
From: Srinivas Kandagatla srinivas.kandagatla@linaro.org
Move ngd platform driver out of loop so that it registers only once.
Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org --- drivers/slimbus/qcom-ngd-ctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c index f48a06e..f7490d7 100644 --- a/drivers/slimbus/qcom-ngd-ctrl.c +++ b/drivers/slimbus/qcom-ngd-ctrl.c @@ -1355,7 +1355,6 @@ static int of_qcom_slim_ngd_register(struct device *parent, ngd->base = ctrl->base + ngd->id * data->offset + (ngd->id - 1) * data->size; ctrl->ngd = ngd; - platform_driver_register(&qcom_slim_ngd_driver);
return 0; } @@ -1447,6 +1446,7 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev) init_completion(&ctrl->reconf); init_completion(&ctrl->qmi.qmi_comp);
+ platform_driver_register(&qcom_slim_ngd_driver); return of_qcom_slim_ngd_register(dev, ctrl); }
participants (1)
-
srinivas.kandagatla＠linaro.org