Re: [PATCH v4 7/8] slimbus: qcom-ngd-ctrl: Remove data channels on stream disable

[Srinivas Kandagatla]

On 9/24/26 2:24 PM, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@xxxxxxx>
> 
> qcom_slim_ngd_xfer_msg() silently returns success for every core
> reconfiguration message, so the generic channel removal done by
> slim_stream_disable() never reaches the ADSP SLIMbus master. The
> master therefore keeps the channels of a closed stream. When the next
> stream defines the same channel numbers again, the WCD9340 capture
> completes without any error but records only zeros; removing the
> channels explicitly when the stream is disabled makes it work again.
> 
> Implement the disable_stream controller op with the sequence the
> downstream NGD driver sends for channel removal, CHAN_CTRL(REMOVE)
> listing the stream's channels followed by RECONFIG_NOW. Like
> downstream, treat -EREMOTEIO from the removal as success: it means the
> controller is being restarted and the channels are gone anyway.
> 
> Based on work done by Casey Tunturi.
> 
> Assisted-by: Claude:claude-fable-5-1
> Signed-off-by: David Heidelberg <david@xxxxxxx>
> ---

there is an identical patch on the list,
https://lore.kernel.org/linux-arm-msm/247e4ce7-1ba2-43b8-8a11-ec70f99a4fc1@xxxxxxxxxx/T/#m3b50aa43a6493f8d3b607b1607b37bf14b199f69

--srini


>  drivers/slimbus/qcom-ngd-ctrl.c | 81 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
> index eb0ccb31e9074..0a4859466c82d 100644
> --- a/drivers/slimbus/qcom-ngd-ctrl.c
> +++ b/drivers/slimbus/qcom-ngd-ctrl.c
> @@ -1081,16 +1081,96 @@ static int qcom_slim_ngd_enable_stream(struct slim_stream_runtime *rt)
>  		slim_free_txn_tid(ctrl, &txn);
>  		dev_err(&sdev->dev, "TX timed out:MC:0x%x,mt:0x%x", txn.mc,
>  				txn.mt);
>  	}
>  
>  	return ret;
>  }
>  
> +/* CHAN_CTRL operation, bits [7:6] of the first payload byte */
> +#define SLIM_USR_CHAN_CTRL_REMOVE	2
> +
> +static int qcom_slim_ngd_disable_stream(struct slim_stream_runtime *rt)
> +{
> +	struct slim_device *sdev = rt->dev;
> +	struct slim_controller *ctrl = sdev->ctrl;
> +	struct slim_val_inf msg =  {0};
> +	u8 wbuf[SLIM_MSGQ_BUF_LEN];
> +	u8 rbuf[SLIM_MSGQ_BUF_LEN];
> +	struct slim_msg_txn txn = {0,};
> +	int i, ret;
> +
> +	txn.mt = SLIM_MSG_MT_DEST_REFERRED_USER;
> +	txn.dt = SLIM_MSG_DEST_LOGICALADDR;
> +	txn.la = SLIM_LA_MGR;
> +	txn.ec = 0;
> +	txn.msg = &msg;
> +	txn.msg->num_bytes = 0;
> +	txn.msg->wbuf = wbuf;
> +	txn.msg->rbuf = rbuf;
> +
> +	/*
> +	 * The NGD drops the core reconfiguration messages that
> +	 * slim_stream_disable() uses to remove channels, so ask the master
> +	 * to remove them the way the downstream NGD driver does:
> +	 * CHAN_CTRL(REMOVE) listing every channel, then RECONFIG_NOW.
> +	 * Without this the master keeps the channels active and a later
> +	 * DEF_ACT_CHAN with the same channel numbers is a no-op, so the
> +	 * data path is never re-armed and a reopened stream moves no data.
> +	 */
> +	/* 5-bit client number, operation in bits [7:6] */
> +	wbuf[txn.msg->num_bytes++] = (SLIM_USR_CHAN_CTRL_REMOVE << 6) |
> +				     (sdev->laddr & 0x1f);
> +
> +	ret = slim_alloc_txn_tid(ctrl, &txn);
> +	if (ret) {
> +		dev_err(&sdev->dev, "Fail to allocate TID\n");
> +		return ret;
> +	}
> +	wbuf[txn.msg->num_bytes++] = txn.tid;
> +
> +	for (i = 0; i < rt->num_ports; i++)
> +		wbuf[txn.msg->num_bytes++] = rt->ports[i].ch.id;
> +
> +	txn.mc = SLIM_USR_MC_CHAN_CTRL;
> +	txn.rl = txn.msg->num_bytes + 4;
> +	ret = qcom_slim_ngd_xfer_msg_sync(ctrl, &txn);
> +	if (ret) {
> +		slim_free_txn_tid(ctrl, &txn);
> +		/* Controller restarting, the channels are gone anyway */
> +		if (ret == -EREMOTEIO)
> +			return 0;
> +		dev_err(&sdev->dev, "TX timed out:MC:0x%x,mt:0x%x", txn.mc,
> +				txn.mt);
> +		return ret;
> +	}
> +
> +	txn.mc = SLIM_USR_MC_RECONFIG_NOW;
> +	txn.msg->num_bytes = 2;
> +	wbuf[1] = sdev->laddr;
> +	txn.rl = txn.msg->num_bytes + 4;
> +
> +	ret = slim_alloc_txn_tid(ctrl, &txn);
> +	if (ret) {
> +		dev_err(ctrl->dev, "Fail to allocate TID\n");
> +		return ret;
> +	}
> +
> +	wbuf[0] = txn.tid;
> +	ret = qcom_slim_ngd_xfer_msg_sync(ctrl, &txn);
> +	if (ret) {
> +		slim_free_txn_tid(ctrl, &txn);
> +		dev_err(&sdev->dev, "TX timed out:MC:0x%x,mt:0x%x", txn.mc,
> +				txn.mt);
> +	}
> +
> +	return ret;
> +}
> +
>  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};
> @@ -1620,16 +1700,17 @@ static int qcom_slim_ngd_ctrl_probe(struct platform_device *pdev)
>  	ctrl->framer.rootfreq = SLIM_ROOT_FREQ >> 3;
>  	ctrl->framer.superfreq =
>  		ctrl->framer.rootfreq / SLIM_CL_PER_SUPERFRAME_DIV8;
>  
>  	ctrl->ctrl.a_framer = &ctrl->framer;
>  	ctrl->ctrl.clkgear = SLIM_MAX_CLK_GEAR;
>  	ctrl->ctrl.get_laddr = qcom_slim_ngd_get_laddr;
>  	ctrl->ctrl.enable_stream = qcom_slim_ngd_enable_stream;
> +	ctrl->ctrl.disable_stream = qcom_slim_ngd_disable_stream;
>  	ctrl->ctrl.xfer_msg = qcom_slim_ngd_xfer_msg;
>  	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);
>