| Message ID | 20240709015818.110384-1-slark_xiao@163.com |
|---|---|
| State | New |
| Headers | show |
| Series | bus: mhi: host: Add firehose support for Foxconn SDX24/SDX55/SDX65 | expand |
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. > > Signed-off-by: Slark Xiao <slark_xiao@163.com> > --- > 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 > + .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? 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 >
At 2024-07-13 23:09:47, "Dmitry Baryshkov" <dmitry.baryshkov@linaro.org> 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. >> >> Signed-off-by: Slark Xiao <slark_xiao@163.com> >> --- >> 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 > >> + .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. >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
On Mon, 15 Jul 2024 at 08:46, Slark Xiao <slark_xiao@163.com> wrote: > > > At 2024-07-13 23:09:47, "Dmitry Baryshkov" <dmitry.baryshkov@linaro.org> 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. > > >> > >> Signed-off-by: Slark Xiao <slark_xiao@163.com> > >> --- > >> 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 > > > > >> + .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/ . > > >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
At 2024-07-15 14:16:57, "Dmitry Baryshkov" <dmitry.baryshkov@linaro.org> wrote: >On Mon, 15 Jul 2024 at 08:46, Slark Xiao <slark_xiao@163.com> wrote: >> >> >> At 2024-07-13 23:09:47, "Dmitry Baryshkov" <dmitry.baryshkov@linaro.org> 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@163.com> >> >> --- >> >> 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 >> >> > >> >> + .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
On Mon, 15 Jul 2024 at 09:27, Slark Xiao <slark_xiao@163.com> wrote: > > > At 2024-07-15 14:16:57, "Dmitry Baryshkov" <dmitry.baryshkov@linaro.org> wrote: > >On Mon, 15 Jul 2024 at 08:46, Slark Xiao <slark_xiao@163.com> wrote: > >> > >> > >> At 2024-07-13 23:09:47, "Dmitry Baryshkov" <dmitry.baryshkov@linaro.org> 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@163.com> > >> >> --- > >> >> 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 > >> > >> > > >> >> + .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 it's common, sounds good to me. > > >> > >> >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
At 2024-07-15 14:27:07, "Slark Xiao" <slark_xiao@163.com> wrote: > >At 2024-07-15 14:16:57, "Dmitry Baryshkov" <dmitry.baryshkov@linaro.org> wrote: >>On Mon, 15 Jul 2024 at 08:46, Slark Xiao <slark_xiao@163.com> wrote: >>> >>> >>> At 2024-07-13 23:09:47, "Dmitry Baryshkov" <dmitry.baryshkov@linaro.org> 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@163.com> >>> >> --- >>> >> 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
On Wed, Jul 24, 2024 at 07:14:40PM +0800, Slark Xiao wrote: > > At 2024-07-15 14:27:07, "Slark Xiao" <slark_xiao@163.com> wrote: > > > >At 2024-07-15 14:16:57, "Dmitry Baryshkov" <dmitry.baryshkov@linaro.org> wrote: > >>On Mon, 15 Jul 2024 at 08:46, Slark Xiao <slark_xiao@163.com> wrote: > >>> > >>> > >>> At 2024-07-13 23:09:47, "Dmitry Baryshkov" <dmitry.baryshkov@linaro.org> 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@163.com> > >>> >> --- > >>> >> 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? > This really sneaked through. I shouldn't have allowed this path. We should bite the bullet and use a standard path for all modems as Dmitry suggested. So the path will become: "qcom/<chip>/<vendor>/" where chip is 'sdx72m' and vendor is 'foxconn'. If you need to use product specific firmware, then "qcom/<chip>/<vendor>/<product>" Since the firmware itself is not upstreamed to linux-firmware, changing the path is not a big deal. - Mani
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", + .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", + .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,
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. Signed-off-by: Slark Xiao <slark_xiao@163.com> --- drivers/bus/mhi/host/pci_generic.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)