bus: mhi: host: Add Foxconn SDX72 related support

Commit Message

Slark Xiao May 10, 2024, 3:26 a.m. UTC

Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
And also, add firehose support since SDX72.

Signed-off-by: Slark Xiao <[e-mail]>
---
 drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

Comments

Manivannan Sadhasivam May 14, 2024, 2:37 p.m. UTC | #1

On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
> And also, add firehose support since SDX72.
> 
> Signed-off-by: Slark Xiao <[e-mail]>
> ---
>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> index 08844ee79654..0fd94c193fc6 100644
> --- a/drivers/bus/mhi/host/pci_generic.c
> +++ b/drivers/bus/mhi/host/pci_generic.c
> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),

This means SDX55 is also supporting FIREHOSE channels, which is not true I
believe.

>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
>  };
> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
>  	.event_cfg = mhi_foxconn_sdx55_events,
>  };
>  
> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
> +	.max_channels = 128,
> +	.timeout_ms = 20000,
> +	.ready_timeout_ms = 50000,
> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
> +	.ch_cfg = mhi_foxconn_sdx55_channels,
> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
> +	.event_cfg = mhi_foxconn_sdx55_events,
> +};
> +
>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>  	.name = "foxconn-sdx24",
>  	.config = &modem_foxconn_sdx55_config,
> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
>  	.sideband_wake = false,
>  };
>  
> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
> +	.name = "foxconn-sdx72",
> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",

What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
it to linux-firmware?

- Mani

Slark Xiao May 15, 2024, 1:29 a.m. UTC | #2

At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
>On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
>> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
>> And also, add firehose support since SDX72.
>> 
>> Signed-off-by: Slark Xiao <[e-mail]>
>> ---
>>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>> 
>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>> index 08844ee79654..0fd94c193fc6 100644
>> --- a/drivers/bus/mhi/host/pci_generic.c
>> +++ b/drivers/bus/mhi/host/pci_generic.c
>> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
>>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
>>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
>>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
>> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
>> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
>
>This means SDX55 is also supporting FIREHOSE channels, which is not true I
>believe.
Actually, I just verified it with my sdx55 and the answer is Yes. These channels
are common settings for Qcom device which support PCIe mode. BTW, the
default settings of Qcom and Quectel support firehose for their sdx55 products.
>
>>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
>>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
>>  };
>> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
>>  	.event_cfg = mhi_foxconn_sdx55_events,
>>  };
>>  
>> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
>> +	.max_channels = 128,
>> +	.timeout_ms = 20000,
>> +	.ready_timeout_ms = 50000,
>> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
>> +	.ch_cfg = mhi_foxconn_sdx55_channels,
>> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
>> +	.event_cfg = mhi_foxconn_sdx55_events,
>> +};
>> +
>>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>>  	.name = "foxconn-sdx24",
>>  	.config = &modem_foxconn_sdx55_config,
>> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
>>  	.sideband_wake = false,
>>  };
>>  
>> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>> +	.name = "foxconn-sdx72",
>> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
>
>What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
>it to linux-firmware?
>
This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
"xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
2: this file may be changed since sdx72 product still under developing in our side. we
may change the base line according to QCOM release.
>- Mani
>
>-- 
>மணிவண்ணன் சதாசிவம்

Slark Xiao May 15, 2024, 1:43 a.m. UTC | #3

Send again with text mode.
At 2024-05-15 09:29:20, "Slark Xiao" <[e-mail]> wrote:
>At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
>>On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
>>> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
>>> And also, add firehose support since SDX72.
>>> 
>>> Signed-off-by: Slark Xiao <[e-mail]>
>>> ---
>>>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
>>>  1 file changed, 31 insertions(+)
>>> 
>>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>>> index 08844ee79654..0fd94c193fc6 100644
>>> --- a/drivers/bus/mhi/host/pci_generic.c
>>> +++ b/drivers/bus/mhi/host/pci_generic.c
>>> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
>>>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
>>>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
>>>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
>>> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
>>> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
>>
>>This means SDX55 is also supporting FIREHOSE channels, which is not true I
>>believe.
>Actually, I just verified it with my sdx55 and the answer is Yes. These channels
>are common settings for Qcom device which support PCIe mode. BTW, the
>default settings of Qcom and Quectel support firehose for their sdx55 products.
>>
>>>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
>>>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
>>>  };
>>> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
>>>  	.event_cfg = mhi_foxconn_sdx55_events,
>>>  };
>>>  
>>> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
>>> +	.max_channels = 128,
>>> +	.timeout_ms = 20000,
>>> +	.ready_timeout_ms = 50000,
>>> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
>>> +	.ch_cfg = mhi_foxconn_sdx55_channels,
>>> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
>>> +	.event_cfg = mhi_foxconn_sdx55_events,
>>> +};
>>> +
>>>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>>>  	.name = "foxconn-sdx24",
>>>  	.config = &modem_foxconn_sdx55_config,
>>> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
>>>  	.sideband_wake = false,
>>>  };
>>>  
>>> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>>> +	.name = "foxconn-sdx72",
>>> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
>>
>>What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
>>it to linux-firmware?
>>
>This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
>"xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
>since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
>2: this file may be changed since sdx72 product still under developing in our side. we
>may change the base line according to QCOM release.
>>- Mani
>>
And I want to say, the os or driver can't recover device with this "edl" file only. This file
only used when device needs to be changed to firehose mode. After that, we need
a tool and a complete firmware package to do the firehose download. Unfortunately,
there is no open source tool to support this download. Even Qcom PCAT tool only
supports their own VID/PID with their own driver. So the function of mhi driver is:
Put device into firehose mode and enumerate the wwan0firehose0 port. The rest
shall be done by enduser themselves. 
>>-- 
>>மணிவண்ணன் சதாசிவம்

Manivannan Sadhasivam May 15, 2024, 7:41 a.m. UTC | #4

+ Qiang

On Wed, May 15, 2024 at 09:29:20AM +0800, Slark Xiao wrote:
> At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
> >> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
> >> And also, add firehose support since SDX72.
> >> 
> >> Signed-off-by: Slark Xiao <[e-mail]>
> >> ---
> >>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
> >>  1 file changed, 31 insertions(+)
> >> 
> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> >> index 08844ee79654..0fd94c193fc6 100644
> >> --- a/drivers/bus/mhi/host/pci_generic.c
> >> +++ b/drivers/bus/mhi/host/pci_generic.c
> >> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
> >>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
> >>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
> >>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
> >> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
> >> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
> >
> >This means SDX55 is also supporting FIREHOSE channels, which is not true I
> >believe.
> Actually, I just verified it with my sdx55 and the answer is Yes. These channels
> are common settings for Qcom device which support PCIe mode. BTW, the
> default settings of Qcom and Quectel support firehose for their sdx55 products.

Qiang, can you please confirm that SDX55 supports FIREHOSE channels?

> >
> >>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
> >>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
> >>  };
> >> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
> >>  	.event_cfg = mhi_foxconn_sdx55_events,
> >>  };
> >>  
> >> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
> >> +	.max_channels = 128,
> >> +	.timeout_ms = 20000,
> >> +	.ready_timeout_ms = 50000,
> >> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
> >> +	.ch_cfg = mhi_foxconn_sdx55_channels,
> >> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
> >> +	.event_cfg = mhi_foxconn_sdx55_events,
> >> +};
> >> +
> >>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
> >>  	.name = "foxconn-sdx24",
> >>  	.config = &modem_foxconn_sdx55_config,
> >> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
> >>  	.sideband_wake = false,
> >>  };
> >>  
> >> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
> >> +	.name = "foxconn-sdx72",
> >> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
> >
> >What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
> >it to linux-firmware?
> >
> This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
> "xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
> since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
> 2: this file may be changed since sdx72 product still under developing in our side. we
> may change the base line according to QCOM release.

Then I would ask you to add support when you have a stable firmware. I do not
want to change the firmware name after some time as it will confuse users.

- Mani

Manivannan Sadhasivam May 15, 2024, 7:43 a.m. UTC | #5

On Wed, May 15, 2024 at 09:43:53AM +0800, Slark Xiao wrote:
> 
> Send again with text mode.
> At 2024-05-15 09:29:20, "Slark Xiao" <[e-mail]> wrote:
> >At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >>On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
> >>> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
> >>> And also, add firehose support since SDX72.
> >>> 
> >>> Signed-off-by: Slark Xiao <[e-mail]>
> >>> ---
> >>>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
> >>>  1 file changed, 31 insertions(+)
> >>> 
> >>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> >>> index 08844ee79654..0fd94c193fc6 100644
> >>> --- a/drivers/bus/mhi/host/pci_generic.c
> >>> +++ b/drivers/bus/mhi/host/pci_generic.c
> >>> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
> >>>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
> >>>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
> >>>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
> >>> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
> >>> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
> >>
> >>This means SDX55 is also supporting FIREHOSE channels, which is not true I
> >>believe.
> >Actually, I just verified it with my sdx55 and the answer is Yes. These channels
> >are common settings for Qcom device which support PCIe mode. BTW, the
> >default settings of Qcom and Quectel support firehose for their sdx55 products.
> >>
> >>>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
> >>>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
> >>>  };
> >>> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
> >>>  	.event_cfg = mhi_foxconn_sdx55_events,
> >>>  };
> >>>  
> >>> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
> >>> +	.max_channels = 128,
> >>> +	.timeout_ms = 20000,
> >>> +	.ready_timeout_ms = 50000,
> >>> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
> >>> +	.ch_cfg = mhi_foxconn_sdx55_channels,
> >>> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
> >>> +	.event_cfg = mhi_foxconn_sdx55_events,
> >>> +};
> >>> +
> >>>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
> >>>  	.name = "foxconn-sdx24",
> >>>  	.config = &modem_foxconn_sdx55_config,
> >>> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
> >>>  	.sideband_wake = false,
> >>>  };
> >>>  
> >>> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
> >>> +	.name = "foxconn-sdx72",
> >>> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
> >>
> >>What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
> >>it to linux-firmware?
> >>
> >This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
> >"xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
> >since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
> >2: this file may be changed since sdx72 product still under developing in our side. we
> >may change the base line according to QCOM release.
> >>- Mani
> >>
> And I want to say, the os or driver can't recover device with this "edl" file only. This file
> only used when device needs to be changed to firehose mode. After that, we need
> a tool and a complete firmware package to do the firehose download. Unfortunately,
> there is no open source tool to support this download. Even Qcom PCAT tool only
> supports their own VID/PID with their own driver. So the function of mhi driver is:
> Put device into firehose mode and enumerate the wwan0firehose0 port. The rest
> shall be done by enduser themselves. 

Can't you use QDL tool? https://github.com/linux-msm/qdl

- Mani

Slark Xiao May 15, 2024, 7:57 a.m. UTC | #6

At 2024-05-15 15:43:15, "Manivannan Sadhasivam" <[e-mail]> wrote:
>On Wed, May 15, 2024 at 09:43:53AM +0800, Slark Xiao wrote:
>> 
>> Send again with text mode.
>> At 2024-05-15 09:29:20, "Slark Xiao" <[e-mail]> wrote:
>> >At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
>> >>On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
>> >>> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
>> >>> And also, add firehose support since SDX72.
>> >>> 
>> >>> Signed-off-by: Slark Xiao <[e-mail]>
>> >>> ---
>> >>>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
>> >>>  1 file changed, 31 insertions(+)
>> >>> 
>> >>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>> >>> index 08844ee79654..0fd94c193fc6 100644
>> >>> --- a/drivers/bus/mhi/host/pci_generic.c
>> >>> +++ b/drivers/bus/mhi/host/pci_generic.c
>> >>> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
>> >>>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
>> >>>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
>> >>>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
>> >>> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
>> >>> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
>> >>
>> >>This means SDX55 is also supporting FIREHOSE channels, which is not true I
>> >>believe.
>> >Actually, I just verified it with my sdx55 and the answer is Yes. These channels
>> >are common settings for Qcom device which support PCIe mode. BTW, the
>> >default settings of Qcom and Quectel support firehose for their sdx55 products.
>> >>
>> >>>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
>> >>>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
>> >>>  };
>> >>> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
>> >>>  	.event_cfg = mhi_foxconn_sdx55_events,
>> >>>  };
>> >>>  
>> >>> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
>> >>> +	.max_channels = 128,
>> >>> +	.timeout_ms = 20000,
>> >>> +	.ready_timeout_ms = 50000,
>> >>> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
>> >>> +	.ch_cfg = mhi_foxconn_sdx55_channels,
>> >>> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
>> >>> +	.event_cfg = mhi_foxconn_sdx55_events,
>> >>> +};
>> >>> +
>> >>>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>> >>>  	.name = "foxconn-sdx24",
>> >>>  	.config = &modem_foxconn_sdx55_config,
>> >>> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
>> >>>  	.sideband_wake = false,
>> >>>  };
>> >>>  
>> >>> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>> >>> +	.name = "foxconn-sdx72",
>> >>> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
>> >>
>> >>What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
>> >>it to linux-firmware?
>> >>
>> >This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
>> >"xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
>> >since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
>> >2: this file may be changed since sdx72 product still under developing in our side. we
>> >may change the base line according to QCOM release.
>> >>- Mani
>> >>
>> And I want to say, the os or driver can't recover device with this "edl" file only. This file
>> only used when device needs to be changed to firehose mode. After that, we need
>> a tool and a complete firmware package to do the firehose download. Unfortunately,
>> there is no open source tool to support this download. Even Qcom PCAT tool only
>> supports their own VID/PID with their own driver. So the function of mhi driver is:
>> Put device into firehose mode and enumerate the wwan0firehose0 port. The rest
>> shall be done by enduser themselves. 
>
>Can't you use QDL tool? https://github.com/linux-msm/qdl
>
>- Mani
>
No. We used this tool for USB only. This tool don't support PCIE device currently.
qdl will call the function usb_open to get device node in 
/dev/bus/usb. But for PCIE device , we will communicate device with wwan0firehose0
port. 
>-- 
>மணிவண்ணன் சதாசிவம்

Slark Xiao May 15, 2024, 8:01 a.m. UTC | #7

At 2024-05-15 15:41:19, "Manivannan Sadhasivam" <[e-mail]> wrote:
>+ Qiang
>
>On Wed, May 15, 2024 at 09:29:20AM +0800, Slark Xiao wrote:
>> At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
>> >On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
>> >> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
>> >> And also, add firehose support since SDX72.
>> >> 
>> >> Signed-off-by: Slark Xiao <[e-mail]>
>> >> ---
>> >>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
>> >>  1 file changed, 31 insertions(+)
>> >> 
>> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>> >> index 08844ee79654..0fd94c193fc6 100644
>> >> --- a/drivers/bus/mhi/host/pci_generic.c
>> >> +++ b/drivers/bus/mhi/host/pci_generic.c
>> >> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
>> >>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
>> >>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
>> >>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
>> >> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
>> >> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
>> >
>> >This means SDX55 is also supporting FIREHOSE channels, which is not true I
>> >believe.
>> Actually, I just verified it with my sdx55 and the answer is Yes. These channels
>> are common settings for Qcom device which support PCIe mode. BTW, the
>> default settings of Qcom and Quectel support firehose for their sdx55 products.
>
>Qiang, can you please confirm that SDX55 supports FIREHOSE channels?
>
>> >
>> >>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
>> >>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
>> >>  };
>> >> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
>> >>  	.event_cfg = mhi_foxconn_sdx55_events,
>> >>  };
>> >>  
>> >> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
>> >> +	.max_channels = 128,
>> >> +	.timeout_ms = 20000,
>> >> +	.ready_timeout_ms = 50000,
>> >> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
>> >> +	.ch_cfg = mhi_foxconn_sdx55_channels,
>> >> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
>> >> +	.event_cfg = mhi_foxconn_sdx55_events,
>> >> +};
>> >> +
>> >>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>> >>  	.name = "foxconn-sdx24",
>> >>  	.config = &modem_foxconn_sdx55_config,
>> >> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
>> >>  	.sideband_wake = false,
>> >>  };
>> >>  
>> >> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>> >> +	.name = "foxconn-sdx72",
>> >> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
>> >
>> >What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
>> >it to linux-firmware?
>> >
>> This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
>> "xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
>> since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
>> 2: this file may be changed since sdx72 product still under developing in our side. we
>> may change the base line according to QCOM release.
>
>Then I would ask you to add support when you have a stable firmware. I do not
>want to change the firmware name after some time as it will confuse users.
>
>- Mani
If a stable firmware must be provided, I think I shall change the folder name from qcom to
fox, do you agree this?
BTW, I need to check if it works after updating 'edl fw' from  xbl_s_devprg_ns.melf to
edl.mbn. 
>
>-- 
>மணிவண்ணன் சதாசிவம்

Qiang Yu May 15, 2024, 11:46 a.m. UTC | #8

On 5/15/2024 3:41 PM, Manivannan Sadhasivam wrote:
> + Qiang
>
> On Wed, May 15, 2024 at 09:29:20AM +0800, Slark Xiao wrote:
>> At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
>>> On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
>>>> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
>>>> And also, add firehose support since SDX72.
>>>>
>>>> Signed-off-by: Slark Xiao <[e-mail]>
>>>> ---
>>>>   drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
>>>>   1 file changed, 31 insertions(+)
>>>>
>>>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>>>> index 08844ee79654..0fd94c193fc6 100644
>>>> --- a/drivers/bus/mhi/host/pci_generic.c
>>>> +++ b/drivers/bus/mhi/host/pci_generic.c
>>>> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
>>>>   	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
>>>>   	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
>>>>   	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
>>>> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
>>>> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
>>> This means SDX55 is also supporting FIREHOSE channels, which is not true I
>>> believe.
>> Actually, I just verified it with my sdx55 and the answer is Yes. These channels
>> are common settings for Qcom device which support PCIe mode. BTW, the
>> default settings of Qcom and Quectel support firehose for their sdx55 products.
> Qiang, can you please confirm that SDX55 supports FIREHOSE channels?
Hi Mani

Yes, SDX55 supports FIREHOSE channels.

Thanks,
Qiang
>
>>>>   	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
>>>>   	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
>>>>   };
>>>> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
>>>>   	.event_cfg = mhi_foxconn_sdx55_events,
>>>>   };
>>>>   
>>>> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
>>>> +	.max_channels = 128,
>>>> +	.timeout_ms = 20000,
>>>> +	.ready_timeout_ms = 50000,
>>>> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
>>>> +	.ch_cfg = mhi_foxconn_sdx55_channels,
>>>> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
>>>> +	.event_cfg = mhi_foxconn_sdx55_events,
>>>> +};
>>>> +
>>>>   static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>>>>   	.name = "foxconn-sdx24",
>>>>   	.config = &modem_foxconn_sdx55_config,
>>>> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
>>>>   	.sideband_wake = false,
>>>>   };
>>>>   
>>>> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>>>> +	.name = "foxconn-sdx72",
>>>> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
>>> What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
>>> it to linux-firmware?
>>>
>> This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
>> "xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
>> since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
>> 2: this file may be changed since sdx72 product still under developing in our side. we
>> may change the base line according to QCOM release.
> Then I would ask you to add support when you have a stable firmware. I do not
> want to change the firmware name after some time as it will confuse users.
>
> - Mani
>

Manivannan Sadhasivam May 15, 2024, 11:52 a.m. UTC | #9

On Wed, May 15, 2024 at 04:01:37PM +0800, Slark Xiao wrote:
> 
> At 2024-05-15 15:41:19, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >+ Qiang
> >
> >On Wed, May 15, 2024 at 09:29:20AM +0800, Slark Xiao wrote:
> >> At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >> >On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
> >> >> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
> >> >> And also, add firehose support since SDX72.
> >> >> 
> >> >> Signed-off-by: Slark Xiao <[e-mail]>
> >> >> ---
> >> >>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
> >> >>  1 file changed, 31 insertions(+)
> >> >> 
> >> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> >> >> index 08844ee79654..0fd94c193fc6 100644
> >> >> --- a/drivers/bus/mhi/host/pci_generic.c
> >> >> +++ b/drivers/bus/mhi/host/pci_generic.c
> >> >> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
> >> >>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
> >> >>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
> >> >>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
> >> >> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
> >> >> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
> >> >
> >> >This means SDX55 is also supporting FIREHOSE channels, which is not true I
> >> >believe.
> >> Actually, I just verified it with my sdx55 and the answer is Yes. These channels
> >> are common settings for Qcom device which support PCIe mode. BTW, the
> >> default settings of Qcom and Quectel support firehose for their sdx55 products.
> >
> >Qiang, can you please confirm that SDX55 supports FIREHOSE channels?
> >
> >> >
> >> >>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
> >> >>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
> >> >>  };
> >> >> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
> >> >>  	.event_cfg = mhi_foxconn_sdx55_events,
> >> >>  };
> >> >>  
> >> >> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
> >> >> +	.max_channels = 128,
> >> >> +	.timeout_ms = 20000,
> >> >> +	.ready_timeout_ms = 50000,
> >> >> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
> >> >> +	.ch_cfg = mhi_foxconn_sdx55_channels,
> >> >> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
> >> >> +	.event_cfg = mhi_foxconn_sdx55_events,
> >> >> +};
> >> >> +
> >> >>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
> >> >>  	.name = "foxconn-sdx24",
> >> >>  	.config = &modem_foxconn_sdx55_config,
> >> >> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
> >> >>  	.sideband_wake = false,
> >> >>  };
> >> >>  
> >> >> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
> >> >> +	.name = "foxconn-sdx72",
> >> >> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
> >> >
> >> >What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
> >> >it to linux-firmware?
> >> >
> >> This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
> >> "xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
> >> since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
> >> 2: this file may be changed since sdx72 product still under developing in our side. we
> >> may change the base line according to QCOM release.
> >
> >Then I would ask you to add support when you have a stable firmware. I do not
> >want to change the firmware name after some time as it will confuse users.
> >
> >- Mani
> If a stable firmware must be provided, I think I shall change the folder name from qcom to
> fox, do you agree this?

Even in that case, where can the user find the firmware?

> BTW, I need to check if it works after updating 'edl fw' from  xbl_s_devprg_ns.melf to
> edl.mbn. 

Okay. IMO, we should upstream the product support only after a stable firmware
release (well stable in the sense a stable name at least).

- Mani

Slark Xiao May 15, 2024, 12:17 p.m. UTC | #10

At 2024-05-15 19:52:39, "Manivannan Sadhasivam" <[e-mail]> wrote:
>On Wed, May 15, 2024 at 04:01:37PM +0800, Slark Xiao wrote:
>> 
>> At 2024-05-15 15:41:19, "Manivannan Sadhasivam" <[e-mail]> wrote:
>> >+ Qiang
>> >
>> >On Wed, May 15, 2024 at 09:29:20AM +0800, Slark Xiao wrote:
>> >> At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
>> >> >On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
>> >> >> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
>> >> >> And also, add firehose support since SDX72.
>> >> >> 
>> >> >> Signed-off-by: Slark Xiao <[e-mail]>
>> >> >> ---
>> >> >>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
>> >> >>  1 file changed, 31 insertions(+)
>> >> >> 
>> >> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>> >> >> index 08844ee79654..0fd94c193fc6 100644
>> >> >> --- a/drivers/bus/mhi/host/pci_generic.c
>> >> >> +++ b/drivers/bus/mhi/host/pci_generic.c
>> >> >> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
>> >> >>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
>> >> >>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
>> >> >>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
>> >> >> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
>> >> >> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
>> >> >
>> >> >This means SDX55 is also supporting FIREHOSE channels, which is not true I
>> >> >believe.
>> >> Actually, I just verified it with my sdx55 and the answer is Yes. These channels
>> >> are common settings for Qcom device which support PCIe mode. BTW, the
>> >> default settings of Qcom and Quectel support firehose for their sdx55 products.
>> >
>> >Qiang, can you please confirm that SDX55 supports FIREHOSE channels?
>> >
>> >> >
>> >> >>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
>> >> >>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
>> >> >>  };
>> >> >> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
>> >> >>  	.event_cfg = mhi_foxconn_sdx55_events,
>> >> >>  };
>> >> >>  
>> >> >> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
>> >> >> +	.max_channels = 128,
>> >> >> +	.timeout_ms = 20000,
>> >> >> +	.ready_timeout_ms = 50000,
>> >> >> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
>> >> >> +	.ch_cfg = mhi_foxconn_sdx55_channels,
>> >> >> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
>> >> >> +	.event_cfg = mhi_foxconn_sdx55_events,
>> >> >> +};
>> >> >> +
>> >> >>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>> >> >>  	.name = "foxconn-sdx24",
>> >> >>  	.config = &modem_foxconn_sdx55_config,
>> >> >> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
>> >> >>  	.sideband_wake = false,
>> >> >>  };
>> >> >>  
>> >> >> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>> >> >> +	.name = "foxconn-sdx72",
>> >> >> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
>> >> >
>> >> >What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
>> >> >it to linux-firmware?
>> >> >
>> >> This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
>> >> "xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
>> >> since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
>> >> 2: this file may be changed since sdx72 product still under developing in our side. we
>> >> may change the base line according to QCOM release.
>> >
>> >Then I would ask you to add support when you have a stable firmware. I do not
>> >want to change the firmware name after some time as it will confuse users.
>> >
>> >- Mani
>> If a stable firmware must be provided, I think I shall change the folder name from qcom to
>> fox, do you agree this?
>
>Even in that case, where can the user find the firmware?
>
I think this edl file could help user let device enter into edl mode(wwan0firehose0).
For PCIE device, there is no opensource tool to support PCIE edl download. If user
could get the tool to do the firehose download, I think it's not hard to get complete firmware
from PC vendor or somewhere else.
>> BTW, I need to check if it works after updating 'edl fw' from  xbl_s_devprg_ns.melf to
>> edl.mbn. 

>
>Okay. IMO, we should upstream the product support only after a stable firmware
>release (well stable in the sense a stable name at least).
>
>- Mani
The check result is we can rename it to align with previous format. Until now, 
I didn't see any mhi device has upstream their firmware to /lib/firmware/qcom folder.
If it's a must, I think we can upstream the edl file later.  Anyway, we hope we can
merge this sdx72 support into 6.10 since customer(Dell) would use this kernel for official
release. But no worry, we can make sure this firehose download method works well in
 our local side.
And also, please help a review about my previous email about fix sdx72 ping failure issue.
There is a fix solution from us. 

Thanks

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

Manivannan Sadhasivam May 16, 2024, 2:23 p.m. UTC | #11

On Wed, May 15, 2024 at 08:17:23PM +0800, Slark Xiao wrote:
> 
> 
> At 2024-05-15 19:52:39, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >On Wed, May 15, 2024 at 04:01:37PM +0800, Slark Xiao wrote:
> >> 
> >> At 2024-05-15 15:41:19, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >> >+ Qiang
> >> >
> >> >On Wed, May 15, 2024 at 09:29:20AM +0800, Slark Xiao wrote:
> >> >> At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >> >> >On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
> >> >> >> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
> >> >> >> And also, add firehose support since SDX72.
> >> >> >> 
> >> >> >> Signed-off-by: Slark Xiao <[e-mail]>
> >> >> >> ---
> >> >> >>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
> >> >> >>  1 file changed, 31 insertions(+)
> >> >> >> 
> >> >> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> >> >> >> index 08844ee79654..0fd94c193fc6 100644
> >> >> >> --- a/drivers/bus/mhi/host/pci_generic.c
> >> >> >> +++ b/drivers/bus/mhi/host/pci_generic.c
> >> >> >> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
> >> >> >>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
> >> >> >>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
> >> >> >>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
> >> >> >> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
> >> >> >> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
> >> >> >
> >> >> >This means SDX55 is also supporting FIREHOSE channels, which is not true I
> >> >> >believe.
> >> >> Actually, I just verified it with my sdx55 and the answer is Yes. These channels
> >> >> are common settings for Qcom device which support PCIe mode. BTW, the
> >> >> default settings of Qcom and Quectel support firehose for their sdx55 products.
> >> >
> >> >Qiang, can you please confirm that SDX55 supports FIREHOSE channels?
> >> >
> >> >> >
> >> >> >>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
> >> >> >>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
> >> >> >>  };
> >> >> >> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
> >> >> >>  	.event_cfg = mhi_foxconn_sdx55_events,
> >> >> >>  };
> >> >> >>  
> >> >> >> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
> >> >> >> +	.max_channels = 128,
> >> >> >> +	.timeout_ms = 20000,
> >> >> >> +	.ready_timeout_ms = 50000,
> >> >> >> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
> >> >> >> +	.ch_cfg = mhi_foxconn_sdx55_channels,
> >> >> >> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
> >> >> >> +	.event_cfg = mhi_foxconn_sdx55_events,
> >> >> >> +};
> >> >> >> +
> >> >> >>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
> >> >> >>  	.name = "foxconn-sdx24",
> >> >> >>  	.config = &modem_foxconn_sdx55_config,
> >> >> >> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
> >> >> >>  	.sideband_wake = false,
> >> >> >>  };
> >> >> >>  
> >> >> >> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
> >> >> >> +	.name = "foxconn-sdx72",
> >> >> >> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
> >> >> >
> >> >> >What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
> >> >> >it to linux-firmware?
> >> >> >
> >> >> This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
> >> >> "xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
> >> >> since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
> >> >> 2: this file may be changed since sdx72 product still under developing in our side. we
> >> >> may change the base line according to QCOM release.
> >> >
> >> >Then I would ask you to add support when you have a stable firmware. I do not
> >> >want to change the firmware name after some time as it will confuse users.
> >> >
> >> >- Mani
> >> If a stable firmware must be provided, I think I shall change the folder name from qcom to
> >> fox, do you agree this?
> >
> >Even in that case, where can the user find the firmware?
> >
> I think this edl file could help user let device enter into edl mode(wwan0firehose0).
> For PCIE device, there is no opensource tool to support PCIE edl download. If user
> could get the tool to do the firehose download, I think it's not hard to get complete firmware
> from PC vendor or somewhere else.

I was told that Qcom will upstream the PCI support for QDL in the coming weeks.
Once that happens (even if a PR), I'll share that with you. Please test it and
let me know if that works or not.

And for entering EDL mode, we have recently added support to trigger EDL mode
from host [1]. Could you also test that? You just need to add `edl_trigger =
true` to the `mhi_pci_dev_info` struct of SDX72 and trigger EDL mode from host
by:

echo 1 > /sys/bus/mhi/devices/.../trigger_edl

> >> BTW, I need to check if it works after updating 'edl fw' from  xbl_s_devprg_ns.melf to
> >> edl.mbn. 
> 
> >
> >Okay. IMO, we should upstream the product support only after a stable firmware
> >release (well stable in the sense a stable name at least).
> >
> >- Mani
> The check result is we can rename it to align with previous format. Until now, 
> I didn't see any mhi device has upstream their firmware to /lib/firmware/qcom folder.

It is not mandatory, but it is a best practise that I recently started asking
for.

> If it's a must, I think we can upstream the edl file later.  Anyway, we hope we can
> merge this sdx72 support into 6.10 since customer(Dell) would use this kernel for official
> release. But no worry, we can make sure this firehose download method works well in
>  our local side.
> And also, please help a review about my previous email about fix sdx72 ping failure issue.
> There is a fix solution from us. 
> 

Qiang is working on that.

- Mani

[1] https://lore.kernel.org/mhi/[e-mail]/

Slark Xiao May 17, 2024, 1:09 a.m. UTC | #12

At 2024-05-16 22:23:46, "Manivannan Sadhasivam" <[e-mail]> wrote:
>On Wed, May 15, 2024 at 08:17:23PM +0800, Slark Xiao wrote:
>> 
>> 
>> At 2024-05-15 19:52:39, "Manivannan Sadhasivam" <[e-mail]> wrote:
>> >On Wed, May 15, 2024 at 04:01:37PM +0800, Slark Xiao wrote:
>> >> 
>> >> At 2024-05-15 15:41:19, "Manivannan Sadhasivam" <[e-mail]> wrote:
>> >> >+ Qiang
>> >> >
>> >> >On Wed, May 15, 2024 at 09:29:20AM +0800, Slark Xiao wrote:
>> >> >> At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
>> >> >> >On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
>> >> >> >> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
>> >> >> >> And also, add firehose support since SDX72.
>> >> >> >> 
>> >> >> >> Signed-off-by: Slark Xiao <[e-mail]>
>> >> >> >> ---
>> >> >> >>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
>> >> >> >>  1 file changed, 31 insertions(+)
>> >> >> >> 
>> >> >> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>> >> >> >> index 08844ee79654..0fd94c193fc6 100644
>> >> >> >> --- a/drivers/bus/mhi/host/pci_generic.c
>> >> >> >> +++ b/drivers/bus/mhi/host/pci_generic.c
>> >> >> >> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
>> >> >> >>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
>> >> >> >>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
>> >> >> >>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
>> >> >> >> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
>> >> >> >> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
>> >> >> >
>> >> >> >This means SDX55 is also supporting FIREHOSE channels, which is not true I
>> >> >> >believe.
>> >> >> Actually, I just verified it with my sdx55 and the answer is Yes. These channels
>> >> >> are common settings for Qcom device which support PCIe mode. BTW, the
>> >> >> default settings of Qcom and Quectel support firehose for their sdx55 products.
>> >> >
>> >> >Qiang, can you please confirm that SDX55 supports FIREHOSE channels?
>> >> >
>> >> >> >
>> >> >> >>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
>> >> >> >>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
>> >> >> >>  };
>> >> >> >> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
>> >> >> >>  	.event_cfg = mhi_foxconn_sdx55_events,
>> >> >> >>  };
>> >> >> >>  
>> >> >> >> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
>> >> >> >> +	.max_channels = 128,
>> >> >> >> +	.timeout_ms = 20000,
>> >> >> >> +	.ready_timeout_ms = 50000,
>> >> >> >> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
>> >> >> >> +	.ch_cfg = mhi_foxconn_sdx55_channels,
>> >> >> >> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
>> >> >> >> +	.event_cfg = mhi_foxconn_sdx55_events,
>> >> >> >> +};
>> >> >> >> +
>> >> >> >>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>> >> >> >>  	.name = "foxconn-sdx24",
>> >> >> >>  	.config = &modem_foxconn_sdx55_config,
>> >> >> >> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
>> >> >> >>  	.sideband_wake = false,
>> >> >> >>  };
>> >> >> >>  
>> >> >> >> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>> >> >> >> +	.name = "foxconn-sdx72",
>> >> >> >> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
>> >> >> >
>> >> >> >What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
>> >> >> >it to linux-firmware?
>> >> >> >
>> >> >> This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
>> >> >> "xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
>> >> >> since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
>> >> >> 2: this file may be changed since sdx72 product still under developing in our side. we
>> >> >> may change the base line according to QCOM release.
>> >> >
>> >> >Then I would ask you to add support when you have a stable firmware. I do not
>> >> >want to change the firmware name after some time as it will confuse users.
>> >> >
>> >> >- Mani
>> >> If a stable firmware must be provided, I think I shall change the folder name from qcom to
>> >> fox, do you agree this?
>> >
>> >Even in that case, where can the user find the firmware?
>> >
>> I think this edl file could help user let device enter into edl mode(wwan0firehose0).
>> For PCIE device, there is no opensource tool to support PCIE edl download. If user
>> could get the tool to do the firehose download, I think it's not hard to get complete firmware
>> from PC vendor or somewhere else.
>
>I was told that Qcom will upstream the PCI support for QDL in the coming weeks.
>Once that happens (even if a PR), I'll share that with you. Please test it and
>let me know if that works or not.
>
Sure. But I think this shall not the block cause for merging this patch, right?
Before that PR, we have verified the firehose function in our local with our
firehose tool which is not open. 
>And for entering EDL mode, we have recently added support to trigger EDL mode
>from host [1]. Could you also test that? You just need to add `edl_trigger =
>true` to the `mhi_pci_dev_info` struct of SDX72 and trigger EDL mode from host
>by:
>
>echo 1 > /sys/bus/mhi/devices/.../trigger_edl
>
Do you remember that I told you I want to merge such function from qualcomm driver
in last year? I merge the commit from QUD driver in my local. Actually it's same as the
commit [1], it's called "force_edl". And sure, the result is yes, it works well.

>> >> BTW, I need to check if it works after updating 'edl fw' from  xbl_s_devprg_ns.melf to
>> >> edl.mbn. 
>> 
>> >
>> >Okay. IMO, we should upstream the product support only after a stable firmware
>> >release (well stable in the sense a stable name at least).
>> >
>> >- Mani
>> The check result is we can rename it to align with previous format. Until now, 
>> I didn't see any mhi device has upstream their firmware to /lib/firmware/qcom folder.
>
>It is not mandatory, but it is a best practise that I recently started asking
>for.
>
>> If it's a must, I think we can upstream the edl file later.  Anyway, we hope we can
>> merge this sdx72 support into 6.10 since customer(Dell) would use this kernel for official
>> release. But no worry, we can make sure this firehose download method works well in
>>  our local side.
>> And also, please help a review about my previous email about fix sdx72 ping failure issue.
>> There is a fix solution from us. 
>> 
>
>Qiang is working on that.
Good to hear that. BTW, may I know the feature merge window in V6.10? I don't worry about
merge window of the network fix commit, since it's a fix with higher priority. But I want to
merge the basic support of my SDX72 before merge window close. This is important for us.

Thanks!
>
>- Mani
>
>[1] https://lore.kernel.org/mhi/[e-mail]/
>
>-- 
>மணிவண்ணன் சதாசிவம்

Slark Xiao May 17, 2024, 9:59 a.m. UTC | #13

At 2024-05-17 09:09:05, "Slark Xiao" <[e-mail]> wrote:
>
>At 2024-05-16 22:23:46, "Manivannan Sadhasivam" <[e-mail]> wrote:
>>On Wed, May 15, 2024 at 08:17:23PM +0800, Slark Xiao wrote:
>>> 
>>> 
>>> At 2024-05-15 19:52:39, "Manivannan Sadhasivam" <[e-mail]> wrote:
>>> >On Wed, May 15, 2024 at 04:01:37PM +0800, Slark Xiao wrote:
>>> >> 
>>> >> At 2024-05-15 15:41:19, "Manivannan Sadhasivam" <[e-mail]> wrote:
>>> >> >+ Qiang
>>> >> >
>>> >> >On Wed, May 15, 2024 at 09:29:20AM +0800, Slark Xiao wrote:
>>> >> >> At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
>>> >> >> >On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
>>> >> >> >> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
>>> >> >> >> And also, add firehose support since SDX72.
>>> >> >> >> 
>>> >> >> >> Signed-off-by: Slark Xiao <[e-mail]>
>>> >> >> >> ---
>>> >> >> >>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
>>> >> >> >>  1 file changed, 31 insertions(+)
>>> >> >> >> 
>>> >> >> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>>> >> >> >> index 08844ee79654..0fd94c193fc6 100644
>>> >> >> >> --- a/drivers/bus/mhi/host/pci_generic.c
>>> >> >> >> +++ b/drivers/bus/mhi/host/pci_generic.c
>>> >> >> >> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
>>> >> >> >>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
>>> >> >> >>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
>>> >> >> >>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
>>> >> >> >> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
>>> >> >> >> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
>>> >> >> >
>>> >> >> >This means SDX55 is also supporting FIREHOSE channels, which is not true I
>>> >> >> >believe.
>>> >> >> Actually, I just verified it with my sdx55 and the answer is Yes. These channels
>>> >> >> are common settings for Qcom device which support PCIe mode. BTW, the
>>> >> >> default settings of Qcom and Quectel support firehose for their sdx55 products.
>>> >> >
>>> >> >Qiang, can you please confirm that SDX55 supports FIREHOSE channels?
>>> >> >
>>> >> >> >
>>> >> >> >>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
>>> >> >> >>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
>>> >> >> >>  };
>>> >> >> >> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
>>> >> >> >>  	.event_cfg = mhi_foxconn_sdx55_events,
>>> >> >> >>  };
>>> >> >> >>  
>>> >> >> >> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
>>> >> >> >> +	.max_channels = 128,
>>> >> >> >> +	.timeout_ms = 20000,
>>> >> >> >> +	.ready_timeout_ms = 50000,
>>> >> >> >> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
>>> >> >> >> +	.ch_cfg = mhi_foxconn_sdx55_channels,
>>> >> >> >> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
>>> >> >> >> +	.event_cfg = mhi_foxconn_sdx55_events,
>>> >> >> >> +};
>>> >> >> >> +
>>> >> >> >>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>>> >> >> >>  	.name = "foxconn-sdx24",
>>> >> >> >>  	.config = &modem_foxconn_sdx55_config,
>>> >> >> >> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
>>> >> >> >>  	.sideband_wake = false,
>>> >> >> >>  };
>>> >> >> >>  
>>> >> >> >> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>>> >> >> >> +	.name = "foxconn-sdx72",
>>> >> >> >> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
>>> >> >> >
>>> >> >> >What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
>>> >> >> >it to linux-firmware?
>>> >> >> >
>>> >> >> This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
>>> >> >> "xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
>>> >> >> since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
>>> >> >> 2: this file may be changed since sdx72 product still under developing in our side. we
>>> >> >> may change the base line according to QCOM release.
>>> >> >
>>> >> >Then I would ask you to add support when you have a stable firmware. I do not
>>> >> >want to change the firmware name after some time as it will confuse users.
>>> >> >
>>> >> >- Mani
>>> >> If a stable firmware must be provided, I think I shall change the folder name from qcom to
>>> >> fox, do you agree this?
>>> >
>>> >Even in that case, where can the user find the firmware?
>>> >
>>> I think this edl file could help user let device enter into edl mode(wwan0firehose0).
>>> For PCIE device, there is no opensource tool to support PCIE edl download. If user
>>> could get the tool to do the firehose download, I think it's not hard to get complete firmware
>>> from PC vendor or somewhere else.
>>
>>I was told that Qcom will upstream the PCI support for QDL in the coming weeks.
>>Once that happens (even if a PR), I'll share that with you. Please test it and
>>let me know if that works or not.
>>
>Sure. But I think this shall not the block cause for merging this patch, right?
>Before that PR, we have verified the firehose function in our local with our
>firehose tool which is not open. 
>>And for entering EDL mode, we have recently added support to trigger EDL mode
>>from host [1]. Could you also test that? You just need to add `edl_trigger =
>>true` to the `mhi_pci_dev_info` struct of SDX72 and trigger EDL mode from host
>>by:
>>
>>echo 1 > /sys/bus/mhi/devices/.../trigger_edl
>>
>Do you remember that I told you I want to merge such function from qualcomm driver
>in last year? I merge the commit from QUD driver in my local. Actually it's same as the
>commit [1], it's called "force_edl". And sure, the result is yes, it works well.
>
Latest test, it doesn't work in Linux V6.9 since there is a patch missing. In my local previous
test, there is no mhi_cntrl->edl_trigger condition to set up dev_attr_trigger_edl.
Seems patch [2] is missed.

[2]-https://lore.kernel.org/mhi/[e-mail]/
>>> >> BTW, I need to check if it works after updating 'edl fw' from  xbl_s_devprg_ns.melf to
>>> >> edl.mbn. 
>>> 
>>> >
>>> >Okay. IMO, we should upstream the product support only after a stable firmware
>>> >release (well stable in the sense a stable name at least).
>>> >
>>> >- Mani
>>> The check result is we can rename it to align with previous format. Until now, 
>>> I didn't see any mhi device has upstream their firmware to /lib/firmware/qcom folder.
>>
>>It is not mandatory, but it is a best practise that I recently started asking
>>for.
>>
>>> If it's a must, I think we can upstream the edl file later.  Anyway, we hope we can
>>> merge this sdx72 support into 6.10 since customer(Dell) would use this kernel for official
>>> release. But no worry, we can make sure this firehose download method works well in
>>>  our local side.
>>> And also, please help a review about my previous email about fix sdx72 ping failure issue.
>>> There is a fix solution from us. 
>>> 
>>
>>Qiang is working on that.
>Good to hear that. BTW, may I know the feature merge window in V6.10? I don't worry about
>merge window of the network fix commit, since it's a fix with higher priority. But I want to
>merge the basic support of my SDX72 before merge window close. This is important for us.
>
>Thanks!
>>
>>- Mani
>>
>>[1] https://lore.kernel.org/mhi/[e-mail]/
>>
>>-- 
>>மணிவண்ணன் சதாசிவம்

Manivannan Sadhasivam May 17, 2024, 10:49 a.m. UTC | #14

On Fri, May 17, 2024 at 05:59:03PM +0800, Slark Xiao wrote:
> 
> At 2024-05-17 09:09:05, "Slark Xiao" <[e-mail]> wrote:
> >
> >At 2024-05-16 22:23:46, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >>On Wed, May 15, 2024 at 08:17:23PM +0800, Slark Xiao wrote:
> >>> 
> >>> 
> >>> At 2024-05-15 19:52:39, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >>> >On Wed, May 15, 2024 at 04:01:37PM +0800, Slark Xiao wrote:
> >>> >> 
> >>> >> At 2024-05-15 15:41:19, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >>> >> >+ Qiang
> >>> >> >
> >>> >> >On Wed, May 15, 2024 at 09:29:20AM +0800, Slark Xiao wrote:
> >>> >> >> At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >>> >> >> >On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
> >>> >> >> >> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
> >>> >> >> >> And also, add firehose support since SDX72.
> >>> >> >> >> 
> >>> >> >> >> Signed-off-by: Slark Xiao <[e-mail]>
> >>> >> >> >> ---
> >>> >> >> >>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
> >>> >> >> >>  1 file changed, 31 insertions(+)
> >>> >> >> >> 
> >>> >> >> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> >>> >> >> >> index 08844ee79654..0fd94c193fc6 100644
> >>> >> >> >> --- a/drivers/bus/mhi/host/pci_generic.c
> >>> >> >> >> +++ b/drivers/bus/mhi/host/pci_generic.c
> >>> >> >> >> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
> >>> >> >> >>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
> >>> >> >> >>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
> >>> >> >> >>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
> >>> >> >> >> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
> >>> >> >> >> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
> >>> >> >> >
> >>> >> >> >This means SDX55 is also supporting FIREHOSE channels, which is not true I
> >>> >> >> >believe.
> >>> >> >> Actually, I just verified it with my sdx55 and the answer is Yes. These channels
> >>> >> >> are common settings for Qcom device which support PCIe mode. BTW, the
> >>> >> >> default settings of Qcom and Quectel support firehose for their sdx55 products.
> >>> >> >
> >>> >> >Qiang, can you please confirm that SDX55 supports FIREHOSE channels?
> >>> >> >
> >>> >> >> >
> >>> >> >> >>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
> >>> >> >> >>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
> >>> >> >> >>  };
> >>> >> >> >> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
> >>> >> >> >>  	.event_cfg = mhi_foxconn_sdx55_events,
> >>> >> >> >>  };
> >>> >> >> >>  
> >>> >> >> >> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
> >>> >> >> >> +	.max_channels = 128,
> >>> >> >> >> +	.timeout_ms = 20000,
> >>> >> >> >> +	.ready_timeout_ms = 50000,
> >>> >> >> >> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
> >>> >> >> >> +	.ch_cfg = mhi_foxconn_sdx55_channels,
> >>> >> >> >> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
> >>> >> >> >> +	.event_cfg = mhi_foxconn_sdx55_events,
> >>> >> >> >> +};
> >>> >> >> >> +
> >>> >> >> >>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
> >>> >> >> >>  	.name = "foxconn-sdx24",
> >>> >> >> >>  	.config = &modem_foxconn_sdx55_config,
> >>> >> >> >> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
> >>> >> >> >>  	.sideband_wake = false,
> >>> >> >> >>  };
> >>> >> >> >>  
> >>> >> >> >> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
> >>> >> >> >> +	.name = "foxconn-sdx72",
> >>> >> >> >> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
> >>> >> >> >
> >>> >> >> >What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
> >>> >> >> >it to linux-firmware?
> >>> >> >> >
> >>> >> >> This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
> >>> >> >> "xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
> >>> >> >> since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
> >>> >> >> 2: this file may be changed since sdx72 product still under developing in our side. we
> >>> >> >> may change the base line according to QCOM release.
> >>> >> >
> >>> >> >Then I would ask you to add support when you have a stable firmware. I do not
> >>> >> >want to change the firmware name after some time as it will confuse users.
> >>> >> >
> >>> >> >- Mani
> >>> >> If a stable firmware must be provided, I think I shall change the folder name from qcom to
> >>> >> fox, do you agree this?
> >>> >
> >>> >Even in that case, where can the user find the firmware?
> >>> >
> >>> I think this edl file could help user let device enter into edl mode(wwan0firehose0).
> >>> For PCIE device, there is no opensource tool to support PCIE edl download. If user
> >>> could get the tool to do the firehose download, I think it's not hard to get complete firmware
> >>> from PC vendor or somewhere else.
> >>
> >>I was told that Qcom will upstream the PCI support for QDL in the coming weeks.
> >>Once that happens (even if a PR), I'll share that with you. Please test it and
> >>let me know if that works or not.
> >>
> >Sure. But I think this shall not the block cause for merging this patch, right?
> >Before that PR, we have verified the firehose function in our local with our
> >firehose tool which is not open. 

Yeah, QDL is not a blocker for this device.

> >>And for entering EDL mode, we have recently added support to trigger EDL mode
> >>from host [1]. Could you also test that? You just need to add `edl_trigger =
> >>true` to the `mhi_pci_dev_info` struct of SDX72 and trigger EDL mode from host
> >>by:
> >>
> >>echo 1 > /sys/bus/mhi/devices/.../trigger_edl
> >>
> >Do you remember that I told you I want to merge such function from qualcomm driver
> >in last year? I merge the commit from QUD driver in my local. Actually it's same as the
> >commit [1], it's called "force_edl". And sure, the result is yes, it works well.
> >
> Latest test, it doesn't work in Linux V6.9 since there is a patch missing. In my local previous
> test, there is no mhi_cntrl->edl_trigger condition to set up dev_attr_trigger_edl.
> Seems patch [2] is missed.
> 
> [2]-https://lore.kernel.org/mhi/[e-mail]/

You need to apply the whole series. But anyway, thanks for testing it out.

> >>> >> BTW, I need to check if it works after updating 'edl fw' from  xbl_s_devprg_ns.melf to
> >>> >> edl.mbn. 
> >>> 
> >>> >
> >>> >Okay. IMO, we should upstream the product support only after a stable firmware
> >>> >release (well stable in the sense a stable name at least).
> >>> >
> >>> >- Mani
> >>> The check result is we can rename it to align with previous format. Until now, 
> >>> I didn't see any mhi device has upstream their firmware to /lib/firmware/qcom folder.
> >>
> >>It is not mandatory, but it is a best practise that I recently started asking
> >>for.
> >>
> >>> If it's a must, I think we can upstream the edl file later.  Anyway, we hope we can
> >>> merge this sdx72 support into 6.10 since customer(Dell) would use this kernel for official
> >>> release. But no worry, we can make sure this firehose download method works well in
> >>>  our local side.
> >>> And also, please help a review about my previous email about fix sdx72 ping failure issue.
> >>> There is a fix solution from us. 
> >>> 
> >>
> >>Qiang is working on that.
> >Good to hear that. BTW, may I know the feature merge window in V6.10? I don't worry about
> >merge window of the network fix commit, since it's a fix with higher priority. But I want to
> >merge the basic support of my SDX72 before merge window close. This is important for us.
> >

MHI tree is closed during -rc6, so there is no way this patch can make 6.10.

- Mani

Slark Xiao May 20, 2024, 1:21 a.m. UTC | #15

At 2024-05-17 18:49:47, "Manivannan Sadhasivam" <[e-mail]> wrote:
>On Fri, May 17, 2024 at 05:59:03PM +0800, Slark Xiao wrote:
>> 
>> At 2024-05-17 09:09:05, "Slark Xiao" <[e-mail]> wrote:
>> >
>> >At 2024-05-16 22:23:46, "Manivannan Sadhasivam" <[e-mail]> wrote:
>> >>On Wed, May 15, 2024 at 08:17:23PM +0800, Slark Xiao wrote:
>> >>> 
>> >>> 
>> >>> At 2024-05-15 19:52:39, "Manivannan Sadhasivam" <[e-mail]> wrote:
>> >>> >On Wed, May 15, 2024 at 04:01:37PM +0800, Slark Xiao wrote:
>> >>> >> 
>> >>> >> At 2024-05-15 15:41:19, "Manivannan Sadhasivam" <[e-mail]> wrote:
>> >>> >> >+ Qiang
>> >>> >> >
>> >>> >> >On Wed, May 15, 2024 at 09:29:20AM +0800, Slark Xiao wrote:
>> >>> >> >> At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
>> >>> >> >> >On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
>> >>> >> >> >> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
>> >>> >> >> >> And also, add firehose support since SDX72.
>> >>> >> >> >> 
>> >>> >> >> >> Signed-off-by: Slark Xiao <[e-mail]>
>> >>> >> >> >> ---
>> >>> >> >> >>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
>> >>> >> >> >>  1 file changed, 31 insertions(+)
>> >>> >> >> >> 
>> >>> >> >> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>> >>> >> >> >> index 08844ee79654..0fd94c193fc6 100644
>> >>> >> >> >> --- a/drivers/bus/mhi/host/pci_generic.c
>> >>> >> >> >> +++ b/drivers/bus/mhi/host/pci_generic.c
>> >>> >> >> >> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
>> >>> >> >> >>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
>> >>> >> >> >>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
>> >>> >> >> >>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
>> >>> >> >> >> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
>> >>> >> >> >> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
>> >>> >> >> >
>> >>> >> >> >This means SDX55 is also supporting FIREHOSE channels, which is not true I
>> >>> >> >> >believe.
>> >>> >> >> Actually, I just verified it with my sdx55 and the answer is Yes. These channels
>> >>> >> >> are common settings for Qcom device which support PCIe mode. BTW, the
>> >>> >> >> default settings of Qcom and Quectel support firehose for their sdx55 products.
>> >>> >> >
>> >>> >> >Qiang, can you please confirm that SDX55 supports FIREHOSE channels?
>> >>> >> >
>> >>> >> >> >
>> >>> >> >> >>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
>> >>> >> >> >>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
>> >>> >> >> >>  };
>> >>> >> >> >> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
>> >>> >> >> >>  	.event_cfg = mhi_foxconn_sdx55_events,
>> >>> >> >> >>  };
>> >>> >> >> >>  
>> >>> >> >> >> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
>> >>> >> >> >> +	.max_channels = 128,
>> >>> >> >> >> +	.timeout_ms = 20000,
>> >>> >> >> >> +	.ready_timeout_ms = 50000,
>> >>> >> >> >> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
>> >>> >> >> >> +	.ch_cfg = mhi_foxconn_sdx55_channels,
>> >>> >> >> >> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
>> >>> >> >> >> +	.event_cfg = mhi_foxconn_sdx55_events,
>> >>> >> >> >> +};
>> >>> >> >> >> +
>> >>> >> >> >>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
>> >>> >> >> >>  	.name = "foxconn-sdx24",
>> >>> >> >> >>  	.config = &modem_foxconn_sdx55_config,
>> >>> >> >> >> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
>> >>> >> >> >>  	.sideband_wake = false,
>> >>> >> >> >>  };
>> >>> >> >> >>  
>> >>> >> >> >> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
>> >>> >> >> >> +	.name = "foxconn-sdx72",
>> >>> >> >> >> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
>> >>> >> >> >
>> >>> >> >> >What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
>> >>> >> >> >it to linux-firmware?
>> >>> >> >> >
>> >>> >> >> This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
>> >>> >> >> "xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
>> >>> >> >> since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
>> >>> >> >> 2: this file may be changed since sdx72 product still under developing in our side. we
>> >>> >> >> may change the base line according to QCOM release.
>> >>> >> >
>> >>> >> >Then I would ask you to add support when you have a stable firmware. I do not
>> >>> >> >want to change the firmware name after some time as it will confuse users.
>> >>> >> >
>> >>> >> >- Mani
>> >>> >> If a stable firmware must be provided, I think I shall change the folder name from qcom to
>> >>> >> fox, do you agree this?
>> >>> >
>> >>> >Even in that case, where can the user find the firmware?
>> >>> >
>> >>> I think this edl file could help user let device enter into edl mode(wwan0firehose0).
>> >>> For PCIE device, there is no opensource tool to support PCIE edl download. If user
>> >>> could get the tool to do the firehose download, I think it's not hard to get complete firmware
>> >>> from PC vendor or somewhere else.
>> >>
>> >>I was told that Qcom will upstream the PCI support for QDL in the coming weeks.
>> >>Once that happens (even if a PR), I'll share that with you. Please test it and
>> >>let me know if that works or not.
>> >>
>> >Sure. But I think this shall not the block cause for merging this patch, right?
>> >Before that PR, we have verified the firehose function in our local with our
>> >firehose tool which is not open. 
>
>Yeah, QDL is not a blocker for this device.
If there is no other issue, I will release a V2 version based on the edl file path and name.
Also, I will add the ".edl_trigger=true" for my sdx72 devices.
For previous sdx55 and sdx65 firehose support, I think a new commit would be better 
to cover that.
>
>> >>And for entering EDL mode, we have recently added support to trigger EDL mode
>> >>from host [1]. Could you also test that? You just need to add `edl_trigger =
>> >>true` to the `mhi_pci_dev_info` struct of SDX72 and trigger EDL mode from host
>> >>by:
>> >>
>> >>echo 1 > /sys/bus/mhi/devices/.../trigger_edl
>> >>
>> >Do you remember that I told you I want to merge such function from qualcomm driver
>> >in last year? I merge the commit from QUD driver in my local. Actually it's same as the
>> >commit [1], it's called "force_edl". And sure, the result is yes, it works well.
>> >
>> Latest test, it doesn't work in Linux V6.9 since there is a patch missing. In my local previous
>> test, there is no mhi_cntrl->edl_trigger condition to set up dev_attr_trigger_edl.
>> Seems patch [2] is missed.
>> 
>> [2]-https://lore.kernel.org/mhi/[e-mail]/
>
>You need to apply the whole series. But anyway, thanks for testing it out.
>
>> >>> >> BTW, I need to check if it works after updating 'edl fw' from  xbl_s_devprg_ns.melf to
>> >>> >> edl.mbn. 
>> >>> 
>> >>> >
>> >>> >Okay. IMO, we should upstream the product support only after a stable firmware
>> >>> >release (well stable in the sense a stable name at least).
>> >>> >
>> >>> >- Mani
>> >>> The check result is we can rename it to align with previous format. Until now, 
>> >>> I didn't see any mhi device has upstream their firmware to /lib/firmware/qcom folder.
>> >>
>> >>It is not mandatory, but it is a best practise that I recently started asking
>> >>for.
>> >>
>> >>> If it's a must, I think we can upstream the edl file later.  Anyway, we hope we can
>> >>> merge this sdx72 support into 6.10 since customer(Dell) would use this kernel for official
>> >>> release. But no worry, we can make sure this firehose download method works well in
>> >>>  our local side.
>> >>> And also, please help a review about my previous email about fix sdx72 ping failure issue.
>> >>> There is a fix solution from us. 
>> >>> 
>> >>
>> >>Qiang is working on that.
>> >Good to hear that. BTW, may I know the feature merge window in V6.10? I don't worry about
>> >merge window of the network fix commit, since it's a fix with higher priority. But I want to
>> >merge the basic support of my SDX72 before merge window close. This is important for us.
>> >
>
>MHI tree is closed during -rc6, so there is no way this patch can make 6.10.
I saw you tag mhi-for-6.10 which was created in 4/25,  before linux 6.9-RC6.
But V6.9 was just released in 5/13, and there is a lot of time before V6.10.
Why close the merge window for V6.10 so early?
>
>- Mani
>
>-- 
>மணிவண்ணன் சதாசிவம்

Manivannan Sadhasivam June 5, 2024, 5:57 a.m. UTC | #16

On Mon, May 20, 2024 at 09:21:16AM +0800, Slark Xiao wrote:
> 
> 
> At 2024-05-17 18:49:47, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >On Fri, May 17, 2024 at 05:59:03PM +0800, Slark Xiao wrote:
> >> 
> >> At 2024-05-17 09:09:05, "Slark Xiao" <[e-mail]> wrote:
> >> >
> >> >At 2024-05-16 22:23:46, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >> >>On Wed, May 15, 2024 at 08:17:23PM +0800, Slark Xiao wrote:
> >> >>> 
> >> >>> 
> >> >>> At 2024-05-15 19:52:39, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >> >>> >On Wed, May 15, 2024 at 04:01:37PM +0800, Slark Xiao wrote:
> >> >>> >> 
> >> >>> >> At 2024-05-15 15:41:19, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >> >>> >> >+ Qiang
> >> >>> >> >
> >> >>> >> >On Wed, May 15, 2024 at 09:29:20AM +0800, Slark Xiao wrote:
> >> >>> >> >> At 2024-05-14 22:37:41, "Manivannan Sadhasivam" <[e-mail]> wrote:
> >> >>> >> >> >On Fri, May 10, 2024 at 11:26:57AM +0800, Slark Xiao wrote:
> >> >>> >> >> >> Align with Qcom SDX72, add ready timeout item for Foxconn SDX72.
> >> >>> >> >> >> And also, add firehose support since SDX72.
> >> >>> >> >> >> 
> >> >>> >> >> >> Signed-off-by: Slark Xiao <[e-mail]>
> >> >>> >> >> >> ---
> >> >>> >> >> >>  drivers/bus/mhi/host/pci_generic.c | 31 ++++++++++++++++++++++++++++++
> >> >>> >> >> >>  1 file changed, 31 insertions(+)
> >> >>> >> >> >> 
> >> >>> >> >> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> >> >>> >> >> >> index 08844ee79654..0fd94c193fc6 100644
> >> >>> >> >> >> --- a/drivers/bus/mhi/host/pci_generic.c
> >> >>> >> >> >> +++ b/drivers/bus/mhi/host/pci_generic.c
> >> >>> >> >> >> @@ -399,6 +399,8 @@ static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
> >> >>> >> >> >>  	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
> >> >>> >> >> >>  	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
> >> >>> >> >> >>  	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
> >> >>> >> >> >> +	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
> >> >>> >> >> >> +	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
> >> >>> >> >> >
> >> >>> >> >> >This means SDX55 is also supporting FIREHOSE channels, which is not true I
> >> >>> >> >> >believe.
> >> >>> >> >> Actually, I just verified it with my sdx55 and the answer is Yes. These channels
> >> >>> >> >> are common settings for Qcom device which support PCIe mode. BTW, the
> >> >>> >> >> default settings of Qcom and Quectel support firehose for their sdx55 products.
> >> >>> >> >
> >> >>> >> >Qiang, can you please confirm that SDX55 supports FIREHOSE channels?
> >> >>> >> >
> >> >>> >> >> >
> >> >>> >> >> >>  	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
> >> >>> >> >> >>  	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
> >> >>> >> >> >>  };
> >> >>> >> >> >> @@ -419,6 +421,16 @@ static const struct mhi_controller_config modem_foxconn_sdx55_config = {
> >> >>> >> >> >>  	.event_cfg = mhi_foxconn_sdx55_events,
> >> >>> >> >> >>  };
> >> >>> >> >> >>  
> >> >>> >> >> >> +static const struct mhi_controller_config modem_foxconn_sdx72_config = {
> >> >>> >> >> >> +	.max_channels = 128,
> >> >>> >> >> >> +	.timeout_ms = 20000,
> >> >>> >> >> >> +	.ready_timeout_ms = 50000,
> >> >>> >> >> >> +	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
> >> >>> >> >> >> +	.ch_cfg = mhi_foxconn_sdx55_channels,
> >> >>> >> >> >> +	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
> >> >>> >> >> >> +	.event_cfg = mhi_foxconn_sdx55_events,
> >> >>> >> >> >> +};
> >> >>> >> >> >> +
> >> >>> >> >> >>  static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
> >> >>> >> >> >>  	.name = "foxconn-sdx24",
> >> >>> >> >> >>  	.config = &modem_foxconn_sdx55_config,
> >> >>> >> >> >> @@ -448,6 +460,16 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
> >> >>> >> >> >>  	.sideband_wake = false,
> >> >>> >> >> >>  };
> >> >>> >> >> >>  
> >> >>> >> >> >> +static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
> >> >>> >> >> >> +	.name = "foxconn-sdx72",
> >> >>> >> >> >> +	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
> >> >>> >> >> >
> >> >>> >> >> >What is '.melf'? Is the firmware available somewhere? Did you plan to upstream
> >> >>> >> >> >it to linux-firmware?
> >> >>> >> >> >
> >> >>> >> >> This file similar with "edl.mbn". In SDX72 product, the default "edl" file name is
> >> >>> >> >> "xbl_s_devprg_ns.melf". Currently we don't plan to upstream it to linux-firmware
> >> >>> >> >> since 2 reasons: 1: we share the same fold name sdx72m with qcom or other vendors
> >> >>> >> >> 2: this file may be changed since sdx72 product still under developing in our side. we
> >> >>> >> >> may change the base line according to QCOM release.
> >> >>> >> >
> >> >>> >> >Then I would ask you to add support when you have a stable firmware. I do not
> >> >>> >> >want to change the firmware name after some time as it will confuse users.
> >> >>> >> >
> >> >>> >> >- Mani
> >> >>> >> If a stable firmware must be provided, I think I shall change the folder name from qcom to
> >> >>> >> fox, do you agree this?
> >> >>> >
> >> >>> >Even in that case, where can the user find the firmware?
> >> >>> >
> >> >>> I think this edl file could help user let device enter into edl mode(wwan0firehose0).
> >> >>> For PCIE device, there is no opensource tool to support PCIE edl download. If user
> >> >>> could get the tool to do the firehose download, I think it's not hard to get complete firmware
> >> >>> from PC vendor or somewhere else.
> >> >>
> >> >>I was told that Qcom will upstream the PCI support for QDL in the coming weeks.
> >> >>Once that happens (even if a PR), I'll share that with you. Please test it and
> >> >>let me know if that works or not.
> >> >>
> >> >Sure. But I think this shall not the block cause for merging this patch, right?
> >> >Before that PR, we have verified the firehose function in our local with our
> >> >firehose tool which is not open. 
> >
> >Yeah, QDL is not a blocker for this device.
> If there is no other issue, I will release a V2 version based on the edl file path and name.
> Also, I will add the ".edl_trigger=true" for my sdx72 devices.
> For previous sdx55 and sdx65 firehose support, I think a new commit would be better 
> to cover that.
> >
> >> >>And for entering EDL mode, we have recently added support to trigger EDL mode
> >> >>from host [1]. Could you also test that? You just need to add `edl_trigger =
> >> >>true` to the `mhi_pci_dev_info` struct of SDX72 and trigger EDL mode from host
> >> >>by:
> >> >>
> >> >>echo 1 > /sys/bus/mhi/devices/.../trigger_edl
> >> >>
> >> >Do you remember that I told you I want to merge such function from qualcomm driver
> >> >in last year? I merge the commit from QUD driver in my local. Actually it's same as the
> >> >commit [1], it's called "force_edl". And sure, the result is yes, it works well.
> >> >
> >> Latest test, it doesn't work in Linux V6.9 since there is a patch missing. In my local previous
> >> test, there is no mhi_cntrl->edl_trigger condition to set up dev_attr_trigger_edl.
> >> Seems patch [2] is missed.
> >> 
> >> [2]-https://lore.kernel.org/mhi/[e-mail]/
> >
> >You need to apply the whole series. But anyway, thanks for testing it out.
> >
> >> >>> >> BTW, I need to check if it works after updating 'edl fw' from  xbl_s_devprg_ns.melf to
> >> >>> >> edl.mbn. 
> >> >>> 
> >> >>> >
> >> >>> >Okay. IMO, we should upstream the product support only after a stable firmware
> >> >>> >release (well stable in the sense a stable name at least).
> >> >>> >
> >> >>> >- Mani
> >> >>> The check result is we can rename it to align with previous format. Until now, 
> >> >>> I didn't see any mhi device has upstream their firmware to /lib/firmware/qcom folder.
> >> >>
> >> >>It is not mandatory, but it is a best practise that I recently started asking
> >> >>for.
> >> >>
> >> >>> If it's a must, I think we can upstream the edl file later.  Anyway, we hope we can
> >> >>> merge this sdx72 support into 6.10 since customer(Dell) would use this kernel for official
> >> >>> release. But no worry, we can make sure this firehose download method works well in
> >> >>>  our local side.
> >> >>> And also, please help a review about my previous email about fix sdx72 ping failure issue.
> >> >>> There is a fix solution from us. 
> >> >>> 
> >> >>
> >> >>Qiang is working on that.
> >> >Good to hear that. BTW, may I know the feature merge window in V6.10? I don't worry about
> >> >merge window of the network fix commit, since it's a fix with higher priority. But I want to
> >> >merge the basic support of my SDX72 before merge window close. This is important for us.
> >> >
> >
> >MHI tree is closed during -rc6, so there is no way this patch can make 6.10.
> I saw you tag mhi-for-6.10 which was created in 4/25,  before linux 6.9-RC6.
> But V6.9 was just released in 5/13, and there is a lot of time before V6.10.
> Why close the merge window for V6.10 so early?

I need to send the PR to Greg at the -rc6 timeframe. Greg will mostly close his
tree post -rc6 as he needs to pull from different sub trees before sending the
PR to Linus during merge window. That's the reason.

- Mani

Patch

diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index 08844ee79654..0fd94c193fc6 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -399,6 +399,8 @@  static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
 	MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
 	MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
 	MHI_CHANNEL_CONFIG_DL(33, "DUN", 32, 0),
+	MHI_CHANNEL_CONFIG_UL_FP(34, "FIREHOSE", 32, 0),
+	MHI_CHANNEL_CONFIG_DL_FP(35, "FIREHOSE", 32, 0),
 	MHI_CHANNEL_CONFIG_HW_UL(100, "IP_HW0_MBIM", 128, 2),
 	MHI_CHANNEL_CONFIG_HW_DL(101, "IP_HW0_MBIM", 128, 3),
 };
@@ -419,6 +421,16 @@  static const struct mhi_controller_config modem_foxconn_sdx55_config = {
 	.event_cfg = mhi_foxconn_sdx55_events,
 };
 
+static const struct mhi_controller_config modem_foxconn_sdx72_config = {
+	.max_channels = 128,
+	.timeout_ms = 20000,
+	.ready_timeout_ms = 50000,
+	.num_channels = ARRAY_SIZE(mhi_foxconn_sdx55_channels),
+	.ch_cfg = mhi_foxconn_sdx55_channels,
+	.num_events = ARRAY_SIZE(mhi_foxconn_sdx55_events),
+	.event_cfg = mhi_foxconn_sdx55_events,
+};
+
 static const struct mhi_pci_dev_info mhi_foxconn_sdx24_info = {
 	.name = "foxconn-sdx24",
 	.config = &modem_foxconn_sdx55_config,
@@ -448,6 +460,16 @@  static const struct mhi_pci_dev_info mhi_foxconn_sdx65_info = {
 	.sideband_wake = false,
 };
 
+static const struct mhi_pci_dev_info mhi_foxconn_sdx72_info = {
+	.name = "foxconn-sdx72",
+	.edl = "qcom/sdx72m/xbl_s_devprg_ns.melf",
+	.config = &modem_foxconn_sdx72_config,
+	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
+	.dma_data_width = 32,
+	.mru_default = 32768,
+	.sideband_wake = false,
+};
+
 static const struct mhi_channel_config mhi_mv3x_channels[] = {
 	MHI_CHANNEL_CONFIG_UL(0, "LOOPBACK", 64, 0),
 	MHI_CHANNEL_CONFIG_DL(1, "LOOPBACK", 64, 0),
@@ -680,6 +702,15 @@  static const struct pci_device_id mhi_pci_id_table[] = {
 	/* DW5932e (sdx62), Non-eSIM */
 	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe0f9),
 		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx65_info },
+	/* T99W515 (sdx72) */
+	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe118),
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx72_info },
+	/* DW5934e(sdx72), With eSIM */
+	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe11d),
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx72_info },
+	/* DW5934e(sdx72), Non-eSIM */
+	{ PCI_DEVICE(PCI_VENDOR_ID_FOXCONN, 0xe11e),
+		.driver_data = (kernel_ulong_t) &mhi_foxconn_sdx72_info },
 	/* MV31-W (Cinterion) */
 	{ PCI_DEVICE(PCI_VENDOR_ID_THALES, 0x00b3),
 		.driver_data = (kernel_ulong_t) &mhi_mv31_info },