[PATCH] bus: mhi: host: Add session ID to MHI controller

[PATCH] bus: mhi: host: Add session ID to MHI controller

[Qiang Yu]

Session ID to be used during BHI transfers to recognize a
particular session are currently not being stored in the MHI
controller structure. Store them to allow for tracking and other
future usage.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
 drivers/bus/mhi/host/boot.c | 8 ++++----
 include/linux/mhi.h         | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
index 26d0edd..5bed8b51 100644
--- a/drivers/bus/mhi/host/boot.c
+++ b/drivers/bus/mhi/host/boot.c
@@ -231,7 +231,7 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
 			   dma_addr_t dma_addr,
 			   size_t size)
 {
-	u32 tx_status, val, session_id;
+	u32 tx_status, val;
 	int i, ret;
 	void __iomem *base = mhi_cntrl->bhi;
 	rwlock_t *pm_lock = &mhi_cntrl->pm_lock;
@@ -253,16 +253,16 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
 		goto invalid_pm_state;
 	}
 
-	session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
+	mhi_cntrl->session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
 	dev_dbg(dev, "Starting image download via BHI. Session ID: %u\n",
-		session_id);
+		mhi_cntrl->session_id);
 	mhi_write_reg(mhi_cntrl, base, BHI_STATUS, 0);
 	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_HIGH,
 		      upper_32_bits(dma_addr));
 	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_LOW,
 		      lower_32_bits(dma_addr));
 	mhi_write_reg(mhi_cntrl, base, BHI_IMGSIZE, size);
-	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
+	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, mhi_cntrl->session_id);
 	read_unlock_bh(pm_lock);
 
 	/* Wait for the image download to complete */
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index a5441ad..8b3c934 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -405,6 +405,7 @@ struct mhi_controller {
 	u32 minor_version;
 	u32 serial_number;
 	u32 oem_pk_hash[MHI_MAX_OEM_PK_HASH_SEGMENTS];
+	u32 session_id;
 
 	struct mhi_event *mhi_event;
 	struct mhi_cmd *mhi_cmd;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] bus: mhi: host: Add session ID to MHI controller

[Manivannan Sadhasivam]

On Mon, Oct 10, 2022 at 12:41:10PM +0800, Qiang Yu wrote:
> Session ID to be used during BHI transfers to recognize a
> particular session are currently not being stored in the MHI
> controller structure. Store them to allow for tracking and other
> future usage.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>

Who/what is going to use this session id?

Thanks,
Mani

> ---
>  drivers/bus/mhi/host/boot.c | 8 ++++----
>  include/linux/mhi.h         | 1 +
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
> index 26d0edd..5bed8b51 100644
> --- a/drivers/bus/mhi/host/boot.c
> +++ b/drivers/bus/mhi/host/boot.c
> @@ -231,7 +231,7 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
>  			   dma_addr_t dma_addr,
>  			   size_t size)
>  {
> -	u32 tx_status, val, session_id;
> +	u32 tx_status, val;
>  	int i, ret;
>  	void __iomem *base = mhi_cntrl->bhi;
>  	rwlock_t *pm_lock = &mhi_cntrl->pm_lock;
> @@ -253,16 +253,16 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
>  		goto invalid_pm_state;
>  	}
>  
> -	session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
> +	mhi_cntrl->session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
>  	dev_dbg(dev, "Starting image download via BHI. Session ID: %u\n",
> -		session_id);
> +		mhi_cntrl->session_id);
>  	mhi_write_reg(mhi_cntrl, base, BHI_STATUS, 0);
>  	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_HIGH,
>  		      upper_32_bits(dma_addr));
>  	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_LOW,
>  		      lower_32_bits(dma_addr));
>  	mhi_write_reg(mhi_cntrl, base, BHI_IMGSIZE, size);
> -	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
> +	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, mhi_cntrl->session_id);
>  	read_unlock_bh(pm_lock);
>  
>  	/* Wait for the image download to complete */
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index a5441ad..8b3c934 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -405,6 +405,7 @@ struct mhi_controller {
>  	u32 minor_version;
>  	u32 serial_number;
>  	u32 oem_pk_hash[MHI_MAX_OEM_PK_HASH_SEGMENTS];
> +	u32 session_id;
>  
>  	struct mhi_event *mhi_event;
>  	struct mhi_cmd *mhi_cmd;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> 

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

Re: [PATCH] bus: mhi: host: Add session ID to MHI controller

[Qiang Yu]

On 10/29/2022 1:20 AM, Manivannan Sadhasivam wrote:
> On Mon, Oct 10, 2022 at 12:41:10PM +0800, Qiang Yu wrote:
>> Session ID to be used during BHI transfers to recognize a
>> particular session are currently not being stored in the MHI
>> controller structure. Store them to allow for tracking and other
>> future usage.
>>
>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> Who/what is going to use this session id?
Sometimes, we may find some error logs and want to see what happens on 
device when host is printing these error logs.
Session id can help us find the logs we want.
>
> Thanks,
> Mani
>
>> ---
>>   drivers/bus/mhi/host/boot.c | 8 ++++----
>>   include/linux/mhi.h         | 1 +
>>   2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
>> index 26d0edd..5bed8b51 100644
>> --- a/drivers/bus/mhi/host/boot.c
>> +++ b/drivers/bus/mhi/host/boot.c
>> @@ -231,7 +231,7 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
>>   			   dma_addr_t dma_addr,
>>   			   size_t size)
>>   {
>> -	u32 tx_status, val, session_id;
>> +	u32 tx_status, val;
>>   	int i, ret;
>>   	void __iomem *base = mhi_cntrl->bhi;
>>   	rwlock_t *pm_lock = &mhi_cntrl->pm_lock;
>> @@ -253,16 +253,16 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
>>   		goto invalid_pm_state;
>>   	}
>>   
>> -	session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
>> +	mhi_cntrl->session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
>>   	dev_dbg(dev, "Starting image download via BHI. Session ID: %u\n",
>> -		session_id);
>> +		mhi_cntrl->session_id);
>>   	mhi_write_reg(mhi_cntrl, base, BHI_STATUS, 0);
>>   	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_HIGH,
>>   		      upper_32_bits(dma_addr));
>>   	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_LOW,
>>   		      lower_32_bits(dma_addr));
>>   	mhi_write_reg(mhi_cntrl, base, BHI_IMGSIZE, size);
>> -	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
>> +	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, mhi_cntrl->session_id);
>>   	read_unlock_bh(pm_lock);
>>   
>>   	/* Wait for the image download to complete */
>> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
>> index a5441ad..8b3c934 100644
>> --- a/include/linux/mhi.h
>> +++ b/include/linux/mhi.h
>> @@ -405,6 +405,7 @@ struct mhi_controller {
>>   	u32 minor_version;
>>   	u32 serial_number;
>>   	u32 oem_pk_hash[MHI_MAX_OEM_PK_HASH_SEGMENTS];
>> +	u32 session_id;
>>   
>>   	struct mhi_event *mhi_event;
>>   	struct mhi_cmd *mhi_cmd;
>> -- 
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>
>>

Re: [PATCH] bus: mhi: host: Add session ID to MHI controller

[Manivannan Sadhasivam]

On Mon, Oct 31, 2022 at 04:54:55PM +0800, Qiang Yu wrote:
> 
> On 10/29/2022 1:20 AM, Manivannan Sadhasivam wrote:
> > On Mon, Oct 10, 2022 at 12:41:10PM +0800, Qiang Yu wrote:
> > > Session ID to be used during BHI transfers to recognize a
> > > particular session are currently not being stored in the MHI
> > > controller structure. Store them to allow for tracking and other
> > > future usage.
> > > 
> > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> > Who/what is going to use this session id?
> Sometimes, we may find some error logs and want to see what happens on
> device when host is printing these error logs.
> Session id can help us find the logs we want.

Okay. Please add this info to the commit message.

Thanks,
Mani

> > 
> > Thanks,
> > Mani
> > 
> > > ---
> > >   drivers/bus/mhi/host/boot.c | 8 ++++----
> > >   include/linux/mhi.h         | 1 +
> > >   2 files changed, 5 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
> > > index 26d0edd..5bed8b51 100644
> > > --- a/drivers/bus/mhi/host/boot.c
> > > +++ b/drivers/bus/mhi/host/boot.c
> > > @@ -231,7 +231,7 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
> > >   			   dma_addr_t dma_addr,
> > >   			   size_t size)
> > >   {
> > > -	u32 tx_status, val, session_id;
> > > +	u32 tx_status, val;
> > >   	int i, ret;
> > >   	void __iomem *base = mhi_cntrl->bhi;
> > >   	rwlock_t *pm_lock = &mhi_cntrl->pm_lock;
> > > @@ -253,16 +253,16 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
> > >   		goto invalid_pm_state;
> > >   	}
> > > -	session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
> > > +	mhi_cntrl->session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
> > >   	dev_dbg(dev, "Starting image download via BHI. Session ID: %u\n",
> > > -		session_id);
> > > +		mhi_cntrl->session_id);
> > >   	mhi_write_reg(mhi_cntrl, base, BHI_STATUS, 0);
> > >   	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_HIGH,
> > >   		      upper_32_bits(dma_addr));
> > >   	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_LOW,
> > >   		      lower_32_bits(dma_addr));
> > >   	mhi_write_reg(mhi_cntrl, base, BHI_IMGSIZE, size);
> > > -	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
> > > +	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, mhi_cntrl->session_id);
> > >   	read_unlock_bh(pm_lock);
> > >   	/* Wait for the image download to complete */
> > > diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> > > index a5441ad..8b3c934 100644
> > > --- a/include/linux/mhi.h
> > > +++ b/include/linux/mhi.h
> > > @@ -405,6 +405,7 @@ struct mhi_controller {
> > >   	u32 minor_version;
> > >   	u32 serial_number;
> > >   	u32 oem_pk_hash[MHI_MAX_OEM_PK_HASH_SEGMENTS];
> > > +	u32 session_id;
> > >   	struct mhi_event *mhi_event;
> > >   	struct mhi_cmd *mhi_cmd;
> > > -- 
> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > > a Linux Foundation Collaborative Project
> > > 
> > > 
> 

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

Re: [PATCH] bus: mhi: host: Add session ID to MHI controller

[Manivannan Sadhasivam]

On Tue, Nov 01, 2022 at 05:15:54PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Oct 31, 2022 at 04:54:55PM +0800, Qiang Yu wrote:
> > 
> > On 10/29/2022 1:20 AM, Manivannan Sadhasivam wrote:
> > > On Mon, Oct 10, 2022 at 12:41:10PM +0800, Qiang Yu wrote:
> > > > Session ID to be used during BHI transfers to recognize a
> > > > particular session are currently not being stored in the MHI
> > > > controller structure. Store them to allow for tracking and other
> > > > future usage.
> > > > 
> > > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> > > Who/what is going to use this session id?
> > Sometimes, we may find some error logs and want to see what happens on
> > device when host is printing these error logs.
> > Session id can help us find the logs we want.
> 
> Okay. Please add this info to the commit message.
> 

Wait. I asked who is going to use session_id cached in the MHI controller
structure? session_id is already printed in debug log.

> Thanks,
> Mani
> 
> > > 
> > > Thanks,
> > > Mani
> > > 
> > > > ---
> > > >   drivers/bus/mhi/host/boot.c | 8 ++++----
> > > >   include/linux/mhi.h         | 1 +
> > > >   2 files changed, 5 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
> > > > index 26d0edd..5bed8b51 100644
> > > > --- a/drivers/bus/mhi/host/boot.c
> > > > +++ b/drivers/bus/mhi/host/boot.c
> > > > @@ -231,7 +231,7 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
> > > >   			   dma_addr_t dma_addr,
> > > >   			   size_t size)
> > > >   {
> > > > -	u32 tx_status, val, session_id;
> > > > +	u32 tx_status, val;
> > > >   	int i, ret;
> > > >   	void __iomem *base = mhi_cntrl->bhi;
> > > >   	rwlock_t *pm_lock = &mhi_cntrl->pm_lock;
> > > > @@ -253,16 +253,16 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
> > > >   		goto invalid_pm_state;
> > > >   	}
> > > > -	session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
> > > > +	mhi_cntrl->session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
> > > >   	dev_dbg(dev, "Starting image download via BHI. Session ID: %u\n",
> > > > -		session_id);
> > > > +		mhi_cntrl->session_id);
> > > >   	mhi_write_reg(mhi_cntrl, base, BHI_STATUS, 0);
> > > >   	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_HIGH,
> > > >   		      upper_32_bits(dma_addr));
> > > >   	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_LOW,
> > > >   		      lower_32_bits(dma_addr));
> > > >   	mhi_write_reg(mhi_cntrl, base, BHI_IMGSIZE, size);
> > > > -	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
> > > > +	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, mhi_cntrl->session_id);
> > > >   	read_unlock_bh(pm_lock);
> > > >   	/* Wait for the image download to complete */
> > > > diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> > > > index a5441ad..8b3c934 100644
> > > > --- a/include/linux/mhi.h
> > > > +++ b/include/linux/mhi.h
> > > > @@ -405,6 +405,7 @@ struct mhi_controller {
> > > >   	u32 minor_version;
> > > >   	u32 serial_number;
> > > >   	u32 oem_pk_hash[MHI_MAX_OEM_PK_HASH_SEGMENTS];
> > > > +	u32 session_id;
> > > >   	struct mhi_event *mhi_event;
> > > >   	struct mhi_cmd *mhi_cmd;
> > > > -- 
> > > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > > > a Linux Foundation Collaborative Project
> > > > 
> > > > 
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்
> 

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

Re: [PATCH] bus: mhi: host: Add session ID to MHI controller

[Qiang Yu]

On 11/3/2022 8:55 PM, Manivannan Sadhasivam wrote:
> On Tue, Nov 01, 2022 at 05:15:54PM +0530, Manivannan Sadhasivam wrote:
>> On Mon, Oct 31, 2022 at 04:54:55PM +0800, Qiang Yu wrote:
>>> On 10/29/2022 1:20 AM, Manivannan Sadhasivam wrote:
>>>> On Mon, Oct 10, 2022 at 12:41:10PM +0800, Qiang Yu wrote:
>>>>> Session ID to be used during BHI transfers to recognize a
>>>>> particular session are currently not being stored in the MHI
>>>>> controller structure. Store them to allow for tracking and other
>>>>> future usage.
>>>>>
>>>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>>>> Who/what is going to use this session id?
>>> Sometimes, we may find some error logs and want to see what happens on
>>> device when host is printing these error logs.
>>> Session id can help us find the logs we want.
>> Okay. Please add this info to the commit message.
>>
> Wait. I asked who is going to use session_id cached in the MHI controller
> structure? session_id is already printed in debug log.
When we repro issue, we often write a ramdom session id before host set 
M0, so that the session id can help us find key logs.
To avoid loss of logs containing seesion id, we need to print session id 
in different place. In some cases, we even print session
id every time when host prints logs as prefix.
>
>> Thanks,
>> Mani
>>
>>>> Thanks,
>>>> Mani
>>>>
>>>>> ---
>>>>>    drivers/bus/mhi/host/boot.c | 8 ++++----
>>>>>    include/linux/mhi.h         | 1 +
>>>>>    2 files changed, 5 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
>>>>> index 26d0edd..5bed8b51 100644
>>>>> --- a/drivers/bus/mhi/host/boot.c
>>>>> +++ b/drivers/bus/mhi/host/boot.c
>>>>> @@ -231,7 +231,7 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
>>>>>    			   dma_addr_t dma_addr,
>>>>>    			   size_t size)
>>>>>    {
>>>>> -	u32 tx_status, val, session_id;
>>>>> +	u32 tx_status, val;
>>>>>    	int i, ret;
>>>>>    	void __iomem *base = mhi_cntrl->bhi;
>>>>>    	rwlock_t *pm_lock = &mhi_cntrl->pm_lock;
>>>>> @@ -253,16 +253,16 @@ static int mhi_fw_load_bhi(struct mhi_controller *mhi_cntrl,
>>>>>    		goto invalid_pm_state;
>>>>>    	}
>>>>> -	session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
>>>>> +	mhi_cntrl->session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
>>>>>    	dev_dbg(dev, "Starting image download via BHI. Session ID: %u\n",
>>>>> -		session_id);
>>>>> +		mhi_cntrl->session_id);
>>>>>    	mhi_write_reg(mhi_cntrl, base, BHI_STATUS, 0);
>>>>>    	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_HIGH,
>>>>>    		      upper_32_bits(dma_addr));
>>>>>    	mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_LOW,
>>>>>    		      lower_32_bits(dma_addr));
>>>>>    	mhi_write_reg(mhi_cntrl, base, BHI_IMGSIZE, size);
>>>>> -	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
>>>>> +	mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, mhi_cntrl->session_id);
>>>>>    	read_unlock_bh(pm_lock);
>>>>>    	/* Wait for the image download to complete */
>>>>> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
>>>>> index a5441ad..8b3c934 100644
>>>>> --- a/include/linux/mhi.h
>>>>> +++ b/include/linux/mhi.h
>>>>> @@ -405,6 +405,7 @@ struct mhi_controller {
>>>>>    	u32 minor_version;
>>>>>    	u32 serial_number;
>>>>>    	u32 oem_pk_hash[MHI_MAX_OEM_PK_HASH_SEGMENTS];
>>>>> +	u32 session_id;
>>>>>    	struct mhi_event *mhi_event;
>>>>>    	struct mhi_cmd *mhi_cmd;
>>>>> -- 
>>>>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>>>>> a Linux Foundation Collaborative Project
>>>>>
>>>>>
>> -- 
>> மணிவண்ணன் சதாசிவம்
>>