[PATCH v1 1/2] bus: mhi: host: Import link_id item

[PATCH v1 1/2] bus: mhi: host: Import link_id item

[Slark Xiao]

For SDX72 MBIM mode, it starts data mux id from 112 instead of 0.
This would lead to device can't ping outside successfully.
Also MBIM side would report "bad packet session (112)".
So we add a link id default value for SDX72.

Signed-off-by: Slark Xiao <slark_xiao@163.com>
---
 drivers/bus/mhi/host/pci_generic.c | 3 +++
 include/linux/mhi.h                | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index 0b483c7c76a1..1f9de2730766 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -53,6 +53,7 @@ struct mhi_pci_dev_info {
 	unsigned int dma_data_width;
 	unsigned int mru_default;
 	bool sideband_wake;
+	unsigned int link_default;
 };
 
 #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \
@@ -469,6 +470,7 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
 	.dma_data_width = 32,
 	.mru_default = 32768,
 	.sideband_wake = false,
+	.link_default = 112,
 };
 
 static const struct mhi_channel_config mhi_mv3x_channels[] = {
@@ -1035,6 +1037,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mhi_cntrl->runtime_get = mhi_pci_runtime_get;
 	mhi_cntrl->runtime_put = mhi_pci_runtime_put;
 	mhi_cntrl->mru = info->mru_default;
+	mhi_cntrl->link_id = info->link_default;
 
 	if (info->edl_trigger)
 		mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index b573f15762f8..4da10b99c96e 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -445,6 +445,7 @@ struct mhi_controller {
 	bool wake_set;
 	unsigned long irq_flags;
 	u32 mru;
+	u32 link_id;
 };
 
 /**
-- 
2.25.1

Re: [PATCH v1 1/2] bus: mhi: host: Import link_id item

[Jeffrey Hugo]

$Subject says this is patch 1 of 2, but I don't see a second patch nor a 
cover letter.

On 6/7/2024 4:01 AM, Slark Xiao wrote:
> For SDX72 MBIM mode, it starts data mux id from 112 instead of 0.
> This would lead to device can't ping outside successfully.
> Also MBIM side would report "bad packet session (112)".
> So we add a link id default value for SDX72.
> 
> Signed-off-by: Slark Xiao <slark_xiao@163.com>
> ---
>   drivers/bus/mhi/host/pci_generic.c | 3 +++
>   include/linux/mhi.h                | 1 +
>   2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> index 0b483c7c76a1..1f9de2730766 100644
> --- a/drivers/bus/mhi/host/pci_generic.c
> +++ b/drivers/bus/mhi/host/pci_generic.c
> @@ -53,6 +53,7 @@ struct mhi_pci_dev_info {
>   	unsigned int dma_data_width;
>   	unsigned int mru_default;
>   	bool sideband_wake;
> +	unsigned int link_default;
>   };
>   
>   #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \
> @@ -469,6 +470,7 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>   	.dma_data_width = 32,
>   	.mru_default = 32768,
>   	.sideband_wake = false,
> +	.link_default = 112,
>   };
>   
>   static const struct mhi_channel_config mhi_mv3x_channels[] = {
> @@ -1035,6 +1037,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	mhi_cntrl->runtime_get = mhi_pci_runtime_get;
>   	mhi_cntrl->runtime_put = mhi_pci_runtime_put;
>   	mhi_cntrl->mru = info->mru_default;
> +	mhi_cntrl->link_id = info->link_default;
>   
>   	if (info->edl_trigger)
>   		mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger;
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index b573f15762f8..4da10b99c96e 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -445,6 +445,7 @@ struct mhi_controller {
>   	bool wake_set;
>   	unsigned long irq_flags;
>   	u32 mru;
> +	u32 link_id;
>   };
>   
>   /**

None of this is actually used.  Dead code is generally not accepted.

-Jeff

Re:Re: [PATCH v1 1/2] bus: mhi: host: Import link_id item

[Slark Xiao]

+More maintainer to this first patch list.

At 2024-06-07 23:01:00, "Jeffrey Hugo" <quic_jhugo@quicinc.com> wrote:
>$Subject says this is patch 1 of 2, but I don't see a second patch nor a 
>cover letter.
>
>On 6/7/2024 4:01 AM, Slark Xiao wrote:
>> For SDX72 MBIM mode, it starts data mux id from 112 instead of 0.
>> This would lead to device can't ping outside successfully.
>> Also MBIM side would report "bad packet session (112)".
>> So we add a link id default value for SDX72.
>> 
>> Signed-off-by: Slark Xiao <slark_xiao@163.com>
>> ---
>>   drivers/bus/mhi/host/pci_generic.c | 3 +++
>>   include/linux/mhi.h                | 1 +
>>   2 files changed, 4 insertions(+)
>> 
>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>> index 0b483c7c76a1..1f9de2730766 100644
>> --- a/drivers/bus/mhi/host/pci_generic.c
>> +++ b/drivers/bus/mhi/host/pci_generic.c
>> @@ -53,6 +53,7 @@ struct mhi_pci_dev_info {
>>   	unsigned int dma_data_width;
>>   	unsigned int mru_default;
>>   	bool sideband_wake;
>> +	unsigned int link_default;
>>   };
>>   
>>   #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \
>> @@ -469,6 +470,7 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>>   	.dma_data_width = 32,
>>   	.mru_default = 32768,
>>   	.sideband_wake = false,
>> +	.link_default = 112,
>>   };
>>   
>>   static const struct mhi_channel_config mhi_mv3x_channels[] = {
>> @@ -1035,6 +1037,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>   	mhi_cntrl->runtime_get = mhi_pci_runtime_get;
>>   	mhi_cntrl->runtime_put = mhi_pci_runtime_put;
>>   	mhi_cntrl->mru = info->mru_default;
>> +	mhi_cntrl->link_id = info->link_default;
>>   
>>   	if (info->edl_trigger)
>>   		mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger;
>> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
>> index b573f15762f8..4da10b99c96e 100644
>> --- a/include/linux/mhi.h
>> +++ b/include/linux/mhi.h
>> @@ -445,6 +445,7 @@ struct mhi_controller {
>>   	bool wake_set;
>>   	unsigned long irq_flags;
>>   	u32 mru;
>> +	u32 link_id;
>>   };
>>   
>>   /**
>
>None of this is actually used.  Dead code is generally not accepted.
>
>-Jeff

Re:Re:Re: [PATCH v1 1/2] bus: mhi: host: Import link_id item

[Slark Xiao]

At 2024-06-11 09:24:07, "Slark Xiao" <slark_xiao@163.com> wrote:
>
>+More maintainer to this first patch list.
>
>At 2024-06-07 23:01:00, "Jeffrey Hugo" <quic_jhugo@quicinc.com> wrote:
>>$Subject says this is patch 1 of 2, but I don't see a second patch nor a 
>>cover letter.
>>
Hi Jeffrey,
I added you in another patch just now. Please help take a view on that.
Thanks.

>>On 6/7/2024 4:01 AM, Slark Xiao wrote:
>>> For SDX72 MBIM mode, it starts data mux id from 112 instead of 0.
>>> This would lead to device can't ping outside successfully.
>>> Also MBIM side would report "bad packet session (112)".
>>> So we add a link id default value for SDX72.
>>> 
>>> Signed-off-by: Slark Xiao <slark_xiao@163.com>
>>> ---
>>>   drivers/bus/mhi/host/pci_generic.c | 3 +++
>>>   include/linux/mhi.h                | 1 +
>>>   2 files changed, 4 insertions(+)
>>> 
>>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>>> index 0b483c7c76a1..1f9de2730766 100644
>>> --- a/drivers/bus/mhi/host/pci_generic.c
>>> +++ b/drivers/bus/mhi/host/pci_generic.c
>>> @@ -53,6 +53,7 @@ struct mhi_pci_dev_info {
>>>   	unsigned int dma_data_width;
>>>   	unsigned int mru_default;
>>>   	bool sideband_wake;
>>> +	unsigned int link_default;
>>>   };
>>>   
>>>   #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \
>>> @@ -469,6 +470,7 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>>>   	.dma_data_width = 32,
>>>   	.mru_default = 32768,
>>>   	.sideband_wake = false,
>>> +	.link_default = 112,
>>>   };
>>>   
>>>   static const struct mhi_channel_config mhi_mv3x_channels[] = {
>>> @@ -1035,6 +1037,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>>   	mhi_cntrl->runtime_get = mhi_pci_runtime_get;
>>>   	mhi_cntrl->runtime_put = mhi_pci_runtime_put;
>>>   	mhi_cntrl->mru = info->mru_default;
>>> +	mhi_cntrl->link_id = info->link_default;
>>>   
>>>   	if (info->edl_trigger)
>>>   		mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger;
>>> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
>>> index b573f15762f8..4da10b99c96e 100644
>>> --- a/include/linux/mhi.h
>>> +++ b/include/linux/mhi.h
>>> @@ -445,6 +445,7 @@ struct mhi_controller {
>>>   	bool wake_set;
>>>   	unsigned long irq_flags;
>>>   	u32 mru;
>>> +	u32 link_id;
>>>   };
>>>   
>>>   /**
>>
>>None of this is actually used.  Dead code is generally not accepted.
>>
>>-Jeff

Re: [PATCH v1 1/2] bus: mhi: host: Import link_id item

[kernel test robot]

Hi Slark,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mani-mhi/mhi-next]
[also build test WARNING on linus/master v6.10-rc2 next-20240607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Slark-Xiao/net-wwan-Fix-SDX72-ping-failure-issue/20240607-180828
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git mhi-next
patch link:    https://lore.kernel.org/r/20240607100114.452979-1-slark_xiao%40163.com
patch subject: [PATCH v1 1/2] bus: mhi: host: Import link_id item
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240608/202406081406.LxqJw00r-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240608/202406081406.LxqJw00r-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406081406.LxqJw00r-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/bus/mhi/host/pci_generic.c:57: warning: Function parameter or struct member 'link_default' not described in 'mhi_pci_dev_info'


vim +57 drivers/bus/mhi/host/pci_generic.c

48f98496b1de13 drivers/bus/mhi/host/pci_generic.c Qiang Yu      2024-04-24  32  
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  33  /**
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  34   * struct mhi_pci_dev_info - MHI PCI device specific information
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  35   * @config: MHI controller configuration
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  36   * @name: name of the PCI module
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  37   * @fw: firmware path (if any)
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  38   * @edl: emergency download mode firmware path (if any)
48f98496b1de13 drivers/bus/mhi/host/pci_generic.c Qiang Yu      2024-04-24  39   * @edl_trigger: capable of triggering EDL mode in the device (if supported)
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  40   * @bar_num: PCI base address register to use for MHI MMIO register space
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  41   * @dma_data_width: DMA transfer word size (32 or 64 bits)
5c2c85315948c4 drivers/bus/mhi/pci_generic.c      Richard Laing 2021-07-15  42   * @mru_default: default MRU size for MBIM network packets
56f6f4c4eb2a71 drivers/bus/mhi/pci_generic.c      Bhaumik Bhatt 2021-07-16  43   * @sideband_wake: Devices using dedicated sideband GPIO for wakeup instead
56f6f4c4eb2a71 drivers/bus/mhi/pci_generic.c      Bhaumik Bhatt 2021-07-16  44   *		   of inband wake support (such as sdx24)
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  45   */
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  46  struct mhi_pci_dev_info {
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  47  	const struct mhi_controller_config *config;
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  48  	const char *name;
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  49  	const char *fw;
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  50  	const char *edl;
48f98496b1de13 drivers/bus/mhi/host/pci_generic.c Qiang Yu      2024-04-24  51  	bool edl_trigger;
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  52  	unsigned int bar_num;
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  53  	unsigned int dma_data_width;
5c2c85315948c4 drivers/bus/mhi/pci_generic.c      Richard Laing 2021-07-15  54  	unsigned int mru_default;
56f6f4c4eb2a71 drivers/bus/mhi/pci_generic.c      Bhaumik Bhatt 2021-07-16  55  	bool sideband_wake;
f749b1f6758869 drivers/bus/mhi/host/pci_generic.c Slark Xiao    2024-06-07  56  	unsigned int link_default;
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21 @57  };
855a70c12021bd drivers/bus/mhi/pci_generic.c      Loic Poulain  2020-10-21  58  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 1/2] bus: mhi: host: Import link_id item

[Manivannan Sadhasivam]

On Fri, Jun 07, 2024 at 06:01:14PM +0800, Slark Xiao wrote:
> For SDX72 MBIM mode, it starts data mux id from 112 instead of 0.
> This would lead to device can't ping outside successfully.
> Also MBIM side would report "bad packet session (112)".
> So we add a link id default value for SDX72.
> 

link_id is WWAN specific. But what you are passing is essentially 'mux_id', so
just use the actual name.

> Signed-off-by: Slark Xiao <slark_xiao@163.com>
> ---
>  drivers/bus/mhi/host/pci_generic.c | 3 +++
>  include/linux/mhi.h                | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> index 0b483c7c76a1..1f9de2730766 100644
> --- a/drivers/bus/mhi/host/pci_generic.c
> +++ b/drivers/bus/mhi/host/pci_generic.c
> @@ -53,6 +53,7 @@ struct mhi_pci_dev_info {
>  	unsigned int dma_data_width;
>  	unsigned int mru_default;
>  	bool sideband_wake;
> +	unsigned int link_default;
>  };
>  
>  #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \
> @@ -469,6 +470,7 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>  	.dma_data_width = 32,
>  	.mru_default = 32768,
>  	.sideband_wake = false,
> +	.link_default = 112,

Just use 'mux_id' here also.

>  };
>  
>  static const struct mhi_channel_config mhi_mv3x_channels[] = {
> @@ -1035,6 +1037,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	mhi_cntrl->runtime_get = mhi_pci_runtime_get;
>  	mhi_cntrl->runtime_put = mhi_pci_runtime_put;
>  	mhi_cntrl->mru = info->mru_default;
> +	mhi_cntrl->link_id = info->link_default;
>  
>  	if (info->edl_trigger)
>  		mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger;
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index b573f15762f8..4da10b99c96e 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -445,6 +445,7 @@ struct mhi_controller {
>  	bool wake_set;
>  	unsigned long irq_flags;
>  	u32 mru;
> +	u32 link_id;

Add kdoc comment.

- Mani

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