Re:Re:Re: Re: [PATCH] bus: mhi: host: Add firehose support for Foxconn SDX24/SDX55/SDX65

From: Slark Xiao
Date: Wed Jul 24 2024 - 07:15:30 EST


At 2024-07-15 14:27:07, "Slark Xiao" <slark_xiao@xxxxxxx> wrote:
>
>At 2024-07-15 14:16:57, "Dmitry Baryshkov" <dmitry.baryshkov@xxxxxxxxxx> wrote:
>>On Mon, 15 Jul 2024 at 08:46, Slark Xiao <slark_xiao@xxxxxxx> wrote:
>>>
>>>
>>> At 2024-07-13 23:09:47, "Dmitry Baryshkov" <dmitry.baryshkov@xxxxxxxxxx> wrote:
>>> >On Tue, Jul 09, 2024 at 09:58:18AM GMT, Slark Xiao wrote:
>>> >> Since we implement the FIREHOSE channel support in foxconn mhi
>>> >> channels, that means each product which use this channel config
>>> >> would support FIREHOSE. But according to the trigger_edl feature,
>>> >> we need to enable it by adding '.edl_trigger = true' in device
>>> >> info struct.
>>> >> Also, we update all edl image path from 'qcom' to 'fox' in case of
>>> >> conflicting with other vendors.
>>> >
>>> >Separate patches please. Also don't use "we", just an imerative style:
>>> >do this and that.
>>> >
>>>
>>> Do you mean use 2 patches (1 for enabling trigger edl and 1 for
>>> modifying path)? Though these changes are aimed to make
>>> firehose download successfully.
>>
>>Yes. "Do this. Also do that" is usually a sign that the patch should be split.
>
>Will do a update in next version.
>
>>
>>>
>>> >>
>>> >> Signed-off-by: Slark Xiao <slark_xiao@xxxxxxx>
>>> >> ---
>>> >> drivers/bus/mhi/host/pci_generic.c | 20 ++++++++++++++------
>>> >> 1 file changed, 14 insertions(+), 6 deletions(-)
>>> >>
>>> >> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>>> >> index 14a11880bcea..440609b81e57 100644
>>> >> --- a/drivers/bus/mhi/host/pci_generic.c
>>> >> +++ b/drivers/bus/mhi/host/pci_generic.c
>>> >> @@ -433,8 +433,8 @@ static const struct mhi_controller_config modem_foxconn_sdx72_config = {
>>> >>
>>> >> static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
>>> >> .name = "foxconn-sdx55",
>>> >> - .fw = "qcom/sdx55m/sbl1.mbn",
>>> >> - .edl = "qcom/sdx55m/edl.mbn",
>>> >> + .edl = "fox/sdx55m/prog_firehose_sdx55.mbn",
>>> >
>>> >qcom/sdx55m/foxconn/prog_firehose_sdx55.mbn
>>>
>>> what's your opinion?Mani

This format mismatch with Foxconn SDX72 edl path"fox/sdx72m/edl.mbn".
I think we should align with that changes we just committed.
What's your opinion, Mani?

>>>
>>> >
>>> >> + .edl_trigger = true,
>>> >> .config = &modem_foxconn_sdx55_config,
>>> >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>>> >> .dma_data_width = 32,
>>> >> @@ -444,8 +444,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
>>> >>
>>> >> static const struct mhi_pci_dev_info mhi_foxconn_t99w175_info = {
>>> >> .name = "foxconn-t99w175",
>>> >> - .fw = "qcom/sdx55m/sbl1.mbn",
>>> >> - .edl = "qcom/sdx55m/edl.mbn",
>>> >> + .edl = "fox/sdx55m/prog_firehose_sdx55.mbn",
>>> >
>>> >Is it the same file as the one mentioned in the previous chunk or is it
>>> >different?
>>> >
>>>
>>> They are same for same chip, though we have some variants.
>>
>>Please excuse me, I can't fully understand. So are the files the same or not?
>>
>>There is a simple mental experiment regarding the file names: you
>>should be able to have a single host rootfs, which supports working
>>with all of your modems at the same time, without modifications.
>>So if modem A and modem B might use file foo.bar and the file is the
>>same for all SDX55 modems, it's fine to have it in qcom/sdx55m/ or in
>>qcom/sdx55m/foxconn. If it is different depending on the end-device,
>>it should go to the qcom/sdx55m/foxconn/devname/ .
>>
>For all Foxconn devices designed based on same chip, they can use same
>edl file. This EDL file is a common image of whole FIREHOSE image package
>of difference variants. The difference of Foxconn SDX55 modem A and
>Foxconn SDX55 modem B is APPS image and Modem image. The EDL
>image is a file for putting device into EDL mode.
>So I prefer to use "qcom/sdx55m/foxconn" since Foxconn's device are
>different with other vendors which provide sdx55 devices as well.
>
>>>
>>> >If they are different, then, please,
>>> >
>>> >qcom/sdx55m/foxconn/t99w175/prog_firehose_sdx55.mbn
>>> >
>>> >
>>> >> + .edl_trigger = true,
>>> >> .config = &modem_foxconn_sdx55_config,
>>> >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>>> >> .dma_data_width = 32,
>>> >> @@ -455,8 +455,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_t99w175_info = {
>>> >>
>>> >> static const struct mhi_pci_dev_info mhi_foxconn_dw5930e_info = {
>>> >> .name = "foxconn-dw5930e",
>>> >> - .fw = "qcom/sdx55m/sbl1.mbn",
>>> >> - .edl = "qcom/sdx55m/edl.mbn",
>>> >> + .edl = "fox/sdx55m/prog_firehose_sdx55.mbn",
>>> >> + .edl_trigger = true,
>>> >> .config = &modem_foxconn_sdx55_config,
>>> >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>>> >> .dma_data_width = 32,
>>> >> @@ -466,6 +466,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_dw5930e_info = {
>>> >>
>>> >> static const struct mhi_pci_dev_info mhi_foxconn_t99w368_info = {
>>> >> .name = "foxconn-t99w368",
>>> >> + .edl = "fox/sdx65m/prog_firehose_lite.elf",
>>> >> + .edl_trigger = true,
>>> >> .config = &modem_foxconn_sdx55_config,
>>> >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>>> >> .dma_data_width = 32,
>>> >> @@ -475,6 +477,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_t99w368_info = {
>>> >>
>>> >> static const struct mhi_pci_dev_info mhi_foxconn_t99w373_info = {
>>> >> .name = "foxconn-t99w373",
>>> >> + .edl = "fox/sdx65m/prog_firehose_lite.elf",
>>> >> + .edl_trigger = true,
>>> >> .config = &modem_foxconn_sdx55_config,
>>> >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>>> >> .dma_data_width = 32,
>>> >> @@ -484,6 +488,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_t99w373_info = {
>>> >>
>>> >> static const struct mhi_pci_dev_info mhi_foxconn_t99w510_info = {
>>> >> .name = "foxconn-t99w510",
>>> >> + .edl = "fox/sdx24m/prog_firehose_sdx24.mbn",
>>> >> + .edl_trigger = true,
>>> >> .config = &modem_foxconn_sdx55_config,
>>> >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>>> >> .dma_data_width = 32,
>>> >> @@ -493,6 +499,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_t99w510_info = {
>>> >>
>>> >> static const struct mhi_pci_dev_info mhi_foxconn_dw5932e_info = {
>>> >> .name = "foxconn-dw5932e",
>>> >> + .edl = "fox/sdx65m/prog_firehose_lite.elf",
>>> >> + .edl_trigger = true,
>>> >> .config = &modem_foxconn_sdx55_config,
>>> >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>>> >> .dma_data_width = 32,
>>> >> --
>>> >> 2.25.1
>>> >>
>>> >
>>> >--
>>> >With best wishes
>>> >Dmitry
>>
>>
>>
>>--
>>With best wishes
>>Dmitry