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

[David Heidelberg <david@ixit.cz>]

On 25/09/2026 10:09, Srinivas Kandagatla wrote:
> On 9/24/26 2:24 PM, David Heidelberg via B4 Relay wrote:
>> From: David Heidelberg <david@ixit.cz>
>>
>> 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@ixit.cz>
>> ---
> 
> there is an identical patch on the list,
> https://lore.kernel.org/linux-arm-msm/247e4ce7-1ba2-43b8-8a11-ec70f99a4fc1@linaro.org/T/#m3b50aa43a6493f8d3b607b1607b37bf14b199f69

I was not aware, I'll try to look into it today and address the comments on the 
original one (and keep the credits as LLM likely "got heavily inspired there").

David

> 
> --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);
>>
>