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

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

[Slark Xiao]

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",
+		session_id);
+	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
 
 	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),
-- 
2.25.1

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

[Krishna Chaitanya Chundru]

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",
> +		session_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.
>   
>   	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),

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

[Slark Xiao]

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

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

[Slark Xiao]

At 2025-09-16 11:26:16, "Slark Xiao" <slark_xiao@163.com> wrote:
>
>
>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

Any updates about this commit?


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

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

[Manivannan Sadhasivam]

On Tue, Oct 14, 2025 at 03:50:03PM +0800, Slark Xiao wrote:
> At 2025-09-16 11:26:16, "Slark Xiao" <slark_xiao@163.com> wrote:
> >
> >
> >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
> 
> Any updates about this commit?
> 

Since this session ID behavior is not documented in the MHI/BHI spec, we can't
have this patch.

- Mani

-- 
மணிவண்ணன் சதாசிவம்