Re:Re: [PATCH] bus: mhi: host: Update session id for each suspend and resume

["Slark Xiao" <slark_xiao@163.com>]

At 2025-09-15 15:26:33, "Krishna Chaitanya Chundru" <krishna.chundru@oss.qualcomm.com> wrote:
>
>
>On 9/12/2025 3:38 PM, Slark Xiao wrote:
>> On Qualcomm module side, there is a UART print as below:
>> session id: 0x355fe689 state:2
>> session id: 0x1f478e42 state:5
>> The session id reads from register BHI_IMGTXDB(0x218), and the
>> state indicates the D3 or D0 state.
>> 
>> In Windows side, MHI driver would update this session id for each
>> suspend/resume progress. We benefit from this mechanism since it
>> could help sync each suspend and resume progress between host and
>> device, especially for some suspend issue which needs to take
>> hundreds or thousands cycle. We can easy to figure out which
>> suspend cycle get a problem and what's happened at that time for
>> both host and device because they have same id.
>> 
>> But in Linux side, this session id value would always be 0x0.
>> So we add it for potential debug usage.
>> 
>> Signed-off-by: Slark Xiao <slark_xiao@163.com>
>> ---
>>   drivers/bus/mhi/host/pm.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>> 
>> diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c
>> index 33d92bf2fc3e..a6573f687363 100644
>> --- a/drivers/bus/mhi/host/pm.c
>> +++ b/drivers/bus/mhi/host/pm.c
>> @@ -864,6 +864,13 @@ int mhi_pm_suspend(struct mhi_controller *mhi_cntrl)
>>   	struct device *dev = &mhi_cntrl->mhi_dev->dev;
>>   	enum mhi_pm_state new_state;
>>   	int ret;
>> +	void __iomem *base = mhi_cntrl->bhi;
>> +	u32 session_id;
>> +
>> +	session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
>> +	dev_dbg(dev, "Starting enter suspend, session id: 0x%x\n",
>> +		sessin_id);
>> +	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
>I agree this will help in debugging, but unless it is documented
>in the MHI spec we can't have this. Since in future if there is
>some other purpose for this register we end up facing issues.
>
>if it already part of spec point it in the commit text.
>
>- Krishna Chaitanya.
Hi Krishna,
I am not a member of Qualcomm so I don't have a detailed MHI
spec to describe this register usage.
But I get some information from their Windows MHI driver:

1. There is release note which describe it:
UDE/QMI/win/qcude/installer/ReadMe.txt 
UDE/QCUDE.Standalone.Source.1.00.44.Windows-AnyCPU_ReadMe_1.txt
......
MHI driver 1.1.0.2
  b. Add write to BHI_IMGTXDB for debug purposes.
......

2.From the code side,  there is a same operation for MHI driver
enter into M0:
HostDriver/win/NTAD/MhiHost/Mhi/src/Mhi.c
......
void MhiRequestM0(PMHI_DEV_CTXT MhiCtxt)
{
.....
   /* Set BHI_IMGTXDB */
   KeQuerySystemTime(&randSeed);
   MhiCtxt->SessionID = RtlRandomEx(&randSeed.LowPart);
   MhiTrace(TRACE_LEVEL_ERROR, TRACE_FLAG_MHICONFIG, "NEW SessionID: 0x%x\n", MhiCtxt->SessionID);
   MHI_WRITE_REG(deviceContext->BHIContext.BhiBase, BHI_IMGTXDB, MhiCtxt->SessionID);

   MhiTrace(TRACE_LEVEL_ERROR, TRACE_FLAG_MHICONFIG, "Req -> M0\n");
......

Not sure if above information is enough for this commit.

Thanks
>>   
>>   	if (mhi_cntrl->pm_state == MHI_PM_DISABLE)
>>   		return -EINVAL;
>> @@ -952,6 +959,14 @@ static int __mhi_pm_resume(struct mhi_controller *mhi_cntrl, bool force)
>>   	struct device *dev = &mhi_cntrl->mhi_dev->dev;
>>   	enum mhi_pm_state cur_state;
>>   	int ret;
>> +	void __iomem *base = mhi_cntrl->bhi;
>> +	u32 session_id;
>> +
>> +	session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
>> +	dev_dbg(dev, "Starting enter resume, session id: 0x%x\n",
>> +		session_id);
>> +
>> +	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
>>   
>>   	dev_dbg(dev, "Entered with PM state: %s, MHI state: %s\n",
>>   		to_mhi_pm_state_str(mhi_cntrl->pm_state),