* [PATCH] bus: mhi: host: Update session id for each suspend and resume
@ 2025-09-12 10:08 Slark Xiao
2025-09-15 7:26 ` Krishna Chaitanya Chundru
0 siblings, 1 reply; 5+ messages in thread
From: Slark Xiao @ 2025-09-12 10:08 UTC (permalink / raw)
To: mani; +Cc: mhi, linux-arm-msm, linux-kernel, 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] bus: mhi: host: Update session id for each suspend and resume
2025-09-12 10:08 [PATCH] bus: mhi: host: Update session id for each suspend and resume Slark Xiao
@ 2025-09-15 7:26 ` Krishna Chaitanya Chundru
2025-09-16 3:26 ` Slark Xiao
0 siblings, 1 reply; 5+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-09-15 7:26 UTC (permalink / raw)
To: Slark Xiao, mani; +Cc: mhi, linux-arm-msm, linux-kernel
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),
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re:Re: [PATCH] bus: mhi: host: Update session id for each suspend and resume
2025-09-15 7:26 ` Krishna Chaitanya Chundru
@ 2025-09-16 3:26 ` Slark Xiao
2025-10-14 7:50 ` Slark Xiao
0 siblings, 1 reply; 5+ messages in thread
From: Slark Xiao @ 2025-09-16 3:26 UTC (permalink / raw)
To: Krishna Chaitanya Chundru; +Cc: mani, mhi, linux-arm-msm, linux-kernel
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),
^ permalink raw reply [flat|nested] 5+ messages in thread* Re:Re:Re: [PATCH] bus: mhi: host: Update session id for each suspend and resume
2025-09-16 3:26 ` Slark Xiao
@ 2025-10-14 7:50 ` Slark Xiao
2025-10-14 12:51 ` Manivannan Sadhasivam
0 siblings, 1 reply; 5+ messages in thread
From: Slark Xiao @ 2025-10-14 7:50 UTC (permalink / raw)
To: Krishna Chaitanya Chundru; +Cc: mani, mhi, linux-arm-msm, linux-kernel
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),
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] bus: mhi: host: Update session id for each suspend and resume
2025-10-14 7:50 ` Slark Xiao
@ 2025-10-14 12:51 ` Manivannan Sadhasivam
0 siblings, 0 replies; 5+ messages in thread
From: Manivannan Sadhasivam @ 2025-10-14 12:51 UTC (permalink / raw)
To: Slark Xiao; +Cc: Krishna Chaitanya Chundru, mhi, linux-arm-msm, linux-kernel
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
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-10-14 12:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-12 10:08 [PATCH] bus: mhi: host: Update session id for each suspend and resume Slark Xiao
2025-09-15 7:26 ` Krishna Chaitanya Chundru
2025-09-16 3:26 ` Slark Xiao
2025-10-14 7:50 ` Slark Xiao
2025-10-14 12:51 ` Manivannan Sadhasivam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®