Re: [PATCH 1/2] slimbus: qcom-ngd-ctrl: add Sub System Restart support

[Yassine Oudjana]

In-Reply-To: <20201118170246.16588-2-srinivas.kandagatla@xxxxxxxxxx>

On Wed, 18 Nov 2020 17:02:45 +0000, Srinivas Kandagatla wrote:
> This patch adds SSR(SubSystem Restart) support which includes, synchronisation
> between SSR and QMI server notifications. Also with this patch now NGD is taken
> down by SSR instead of QMI server down notification.
>
> NGD up path now relies on both SSR and QMI notifications and particularly
> sequence of SSR up followed by QMI server up notification.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> ---
>  drivers/slimbus/Kconfig         |  1 +
>  drivers/slimbus/qcom-ngd-ctrl.c | 97 +++++++++++++++++++++++++++++++--
>  2 files changed, 94 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/slimbus/Kconfig b/drivers/slimbus/Kconfig
> index 8cd595148d17..7c950948a9ec 100644
> --- a/drivers/slimbus/Kconfig
> +++ b/drivers/slimbus/Kconfig
> @@ -25,6 +25,7 @@ config SLIM_QCOM_NGD_CTRL
>  	depends on HAS_IOMEM && DMA_ENGINE && NET
>  	depends on ARCH_QCOM || COMPILE_TEST
>  	select QCOM_QMI_HELPERS
> +	select QCOM_RPROC_COMMON
>  	help
>  	  Select driver if Qualcomm's SLIMbus Satellite Non-Generic Device
>  	  Component is programmed using Linux kernel.
> diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
> index 218aefc3531c..f62693653d2b 100644
> --- a/drivers/slimbus/qcom-ngd-ctrl.c
> +++ b/drivers/slimbus/qcom-ngd-ctrl.c
> @@ -13,6 +13,9 @@
>  #include <linux/slimbus.h>
>  #include <linux/delay.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +#include <linux/remoteproc/qcom_rproc.h>
>  #include <linux/of.h>
>  #include <linux/io.h>
>  #include <linux/soc/qcom/qmi.h>
> @@ -155,8 +158,14 @@ struct qcom_slim_ngd_ctrl {
>  	struct qcom_slim_ngd_dma_desc txdesc[QCOM_SLIM_NGD_DESC_NUM];
>  	struct completion reconf;
>  	struct work_struct m_work;
> +	struct work_struct ngd_up_work;
>  	struct workqueue_struct *mwq;
> +	struct completion qmi_up;
>  	spinlock_t tx_buf_lock;
> +	struct mutex tx_lock;
> +	struct mutex ssr_lock;
> +	struct notifier_block nb;
> +	void *notifier;
>  	enum qcom_slim_ngd_state state;
>  	dma_addr_t rx_phys_base;
>  	dma_addr_t tx_phys_base;
> @@ -868,14 +877,18 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller *sctrl,
>  	if (txn->msg && txn->msg->wbuf)
>  		memcpy(puc, txn->msg->wbuf, txn->msg->num_bytes);
>
> +	mutex_lock(&ctrl->tx_lock);
>  	ret = qcom_slim_ngd_tx_msg_post(ctrl, pbuf, txn->rl);
> -	if (ret)
> +	if (ret) {
> +		mutex_unlock(&ctrl->tx_lock);
>  		return ret;
> +	}
>
>  	timeout = wait_for_completion_timeout(&tx_sent, HZ);
>  	if (!timeout) {
>  		dev_err(sctrl->dev, "TX timed out:MC:0x%x,mt:0x%x", txn->mc,
>  					txn->mt);
> +		mutex_unlock(&ctrl->tx_lock);
>  		return -ETIMEDOUT;
>  	}
>
> @@ -884,10 +897,12 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller *sctrl,
>  		if (!timeout) {
>  			dev_err(sctrl->dev, "TX timed out:MC:0x%x,mt:0x%x",
>  				txn->mc, txn->mt);
> +			mutex_unlock(&ctrl->tx_lock);
>  			return -ETIMEDOUT;
>  		}
>  	}
>
> +	mutex_unlock(&ctrl->tx_lock);
>  	return 0;
>  }
>
> @@ -1200,6 +1215,13 @@ static void qcom_slim_ngd_master_worker(struct work_struct *work)
>  	}
>  }
>
> +static int qcom_slim_ngd_update_device_status(struct device *dev, void *null)
> +{
> +	slim_report_absent(to_slim_device(dev));
> +
> +	return 0;
> +}
> +
>  static int qcom_slim_ngd_runtime_resume(struct device *dev)
>  {
>  	struct qcom_slim_ngd_ctrl *ctrl = dev_get_drvdata(dev);
> @@ -1267,7 +1289,7 @@ static int qcom_slim_ngd_qmi_new_server(struct qmi_handle *hdl,
>  	qmi->svc_info.sq_node = service->node;
>  	qmi->svc_info.sq_port = service->port;
>
> -	qcom_slim_ngd_enable(ctrl, true);
> +	complete(&ctrl->qmi_up);
>
>  	return 0;
>  }
> @@ -1280,10 +1302,9 @@ static void qcom_slim_ngd_qmi_del_server(struct qmi_handle *hdl,
>  	struct qcom_slim_ngd_ctrl *ctrl =
>  		container_of(qmi, struct qcom_slim_ngd_ctrl, qmi);
>
> +	reinit_completion(&ctrl->qmi_up);
>  	qmi->svc_info.sq_node = 0;
>  	qmi->svc_info.sq_port = 0;
> -
> -	qcom_slim_ngd_enable(ctrl, false);
>  }
>
>  static struct qmi_ops qcom_slim_ngd_qmi_svc_event_ops = {
> @@ -1333,6 +1354,64 @@ static const struct of_device_id qcom_slim_ngd_dt_match[] = {
>
>  MODULE_DEVICE_TABLE(of, qcom_slim_ngd_dt_match);
>
> +static void qcom_slim_ngd_down(struct qcom_slim_ngd_ctrl *ctrl)
> +{
> +	mutex_lock(&ctrl->ssr_lock);
> +	device_for_each_child(ctrl->ctrl.dev, NULL,
> +			      qcom_slim_ngd_update_device_status);
> +	qcom_slim_ngd_enable(ctrl, false);
> +	mutex_unlock(&ctrl->ssr_lock);
> +}
> +
> +static void qcom_slim_ngd_up_worker(struct work_struct *work)
> +{
> +	struct qcom_slim_ngd_ctrl *ctrl;
> +
> +	ctrl = container_of(work, struct qcom_slim_ngd_ctrl, ngd_up_work);
> +
> +	/* Make sure qmi service is up before continuing */
> +	wait_for_completion_interruptible(&ctrl->qmi_up);
> +
> +	mutex_lock(&ctrl->ssr_lock);
> +	qcom_slim_ngd_enable(ctrl, true);
> +	mutex_unlock(&ctrl->ssr_lock);
> +}
> +
> +static int qcom_slim_ngd_ssr_pdr_notify(struct qcom_slim_ngd_ctrl *ctrl,
> +					unsigned long action)
> +{
> +	switch (action) {
> +        case QCOM_SSR_BEFORE_SHUTDOWN:
> +		/* Make sure the last dma xfer is finished */
> +		mutex_lock(&ctrl->tx_lock);
> +		if (ctrl->state != QCOM_SLIM_NGD_CTRL_DOWN) {
> +			pm_runtime_get_noresume(ctrl->dev);
> +			ctrl->state = QCOM_SLIM_NGD_CTRL_DOWN;
> +			qcom_slim_ngd_down(ctrl);
> +			qcom_slim_ngd_exit_dma(ctrl);
> +		}
> +		mutex_unlock(&ctrl->tx_lock);
> +                break;
> +        case QCOM_SSR_AFTER_POWERUP:
> +		schedule_work(&ctrl->ngd_up_work);
> +		break;
> +        default:
> +                break;
> +        }
> +
> +        return NOTIFY_OK;
> +}
> +
> +static int qcom_slim_ngd_ssr_notify(struct notifier_block *nb,
> +				    unsigned long action,
> +				    void *data)
> +{
> +	struct qcom_slim_ngd_ctrl *ctrl = container_of(nb,
> +					       struct qcom_slim_ngd_ctrl, nb);
> +
> +	return qcom_slim_ngd_ssr_pdr_notify(ctrl, action);
> +}
> +
>  static int of_qcom_slim_ngd_register(struct device *parent,
>  				     struct qcom_slim_ngd_ctrl *ctrl)
>  {
> @@ -1397,6 +1476,7 @@ static int qcom_slim_ngd_probe(struct platform_device *pdev)
>  	}
>
>  	INIT_WORK(&ctrl->m_work, qcom_slim_ngd_master_worker);
> +	INIT_WORK(&ctrl->ngd_up_work, qcom_slim_ngd_up_worker);
>  	ctrl->mwq = create_singlethread_workqueue("ngd_master");
>  	if (!ctrl->mwq) {
>  		dev_err(&pdev->dev, "Failed to start master worker\n");
> @@ -1444,6 +1524,11 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>
> +	ctrl->nb.notifier_call = qcom_slim_ngd_ssr_notify;
> +	ctrl->notifier = qcom_register_ssr_notifier("lpass", &ctrl->nb);
> +	if (IS_ERR(ctrl->notifier))
> +		return PTR_ERR(ctrl->notifier);
> +
>  	ctrl->dev = dev;
>  	ctrl->framer.rootfreq = SLIM_ROOT_FREQ >> 3;
>  	ctrl->framer.superfreq =
> @@ -1457,9 +1542,12 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
>  	ctrl->ctrl.wakeup = NULL;
>  	ctrl->state = QCOM_SLIM_NGD_CTRL_DOWN;
>
> +	mutex_init(&ctrl->tx_lock);
> +	mutex_init(&ctrl->ssr_lock);
>  	spin_lock_init(&ctrl->tx_buf_lock);
>  	init_completion(&ctrl->reconf);
>  	init_completion(&ctrl->qmi.qmi_comp);
> +	init_completion(&ctrl->qmi_up);
>
>  	platform_driver_register(&qcom_slim_ngd_driver);
>  	return of_qcom_slim_ngd_register(dev, ctrl);
> @@ -1477,6 +1565,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);
> +	qcom_unregister_ssr_notifier(ctrl->notifier, &ctrl->nb);
>  	qcom_slim_ngd_enable(ctrl, false);
>  	qcom_slim_ngd_exit_dma(ctrl);
>  	qcom_slim_ngd_qmi_svc_event_deinit(&ctrl->qmi);
> --
> 2.21.0

This makes NGD never come up if probed after ADSP is powered on since it registers its SSR notifier
after ADSP sent its after powerup notification.