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

[Manivannan Sadhasivam]

On Wed, Jul 24, 2024 at 07:14:40PM +0800, Slark Xiao wrote:
> 
> 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?
> 

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

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