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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7036d57d.5904.190b512d00b.Coremail.slark_xiao@163.com>
Date: Mon, 15 Jul 2024 14:27:07 +0800 (CST)
From: "Slark Xiao" <slark_xiao@....com>
To: "Dmitry Baryshkov" <dmitry.baryshkov@...aro.org>
Cc: manivannan.sadhasivam@...aro.org, mhi@...ts.linux.dev, 
	linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re:Re: Re: [PATCH] bus: mhi: host: Add firehose support for Foxconn
 SDX24/SDX55/SDX65


At 2024-07-15 14:16:57, "Dmitry Baryshkov" <dmitry.baryshkov@...aro.org> wrote:
>On Mon, 15 Jul 2024 at 08:46, Slark Xiao <slark_xiao@....com> wrote:
>>
>>
>> At 2024-07-13 23:09:47, "Dmitry Baryshkov" <dmitry.baryshkov@...aro.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@....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