| Message ID | 20250603-standard_elf_image_load_support-v2-1-cce97644e99e@oss.qualcomm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mhi: host: Add standard elf image download functionality | expand |
On 6/6/2025 1:04 AM, Manivannan Sadhasivam wrote: > On Tue, Jun 03, 2025 at 02:05:44AM -0700, Qiang Yu wrote: >> From: Mayank Rana <mayank.rana@oss.qualcomm.com> >> >> Currently, the FBC image is a non-standard ELF file that contains a single >> ELF header, followed by segments for SBL, RDDM, and AMSS. Some devices are >> unable to process this non-standard ELF format and therefore require >> special handling during image loading. >> > > What are those "some devices"? Why are they not able to process this format > which is used across the rest of the Qcom devices? > >> Add standard_elf_image flag to determine whether the device can process >> the non-standard ELF format. If this flag is set, a standard ELF image >> must be loaded, meaning the first 512 KB of the FBC image should be >> skipped when loading the AMSS image over the BHIe interface. > > Please explain what is present in the first 512KiB and why skipping that is > required. > >> Note that >> this flag does not affect the SBL image download process. >> >> Signed-off-by: Mayank Rana <mayank.rana@oss.qualcomm.com> >> Co-developed-by: Qiang Yu <qiang.yu@oss.qualcomm.com> >> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com> >> --- >> Changes in v2: >> - V1 patch is paused because of no user. WLAN team plan to add support for >> new WLAN chip that requires this patch, so send v2. >> - Change author and SOB with new mail address. >> - Reword commit message. >> - Place standard_elf_image flag after wake_set in struct mhi_controller >> - Link to v1: https://lore.kernel.org/mhi/1689907189-21844-1-git-send-email-quic_qianyu@quicinc.com/ >> --- >> drivers/bus/mhi/host/boot.c | 7 +++++++ >> include/linux/mhi.h | 4 ++++ >> 2 files changed, 11 insertions(+) >> >> diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c >> index efa3b6dddf4d2f937535243bd8e8ed32109150a4..f1686a8e0681d49f778838820b44f4c845ddbd1f 100644 >> --- a/drivers/bus/mhi/host/boot.c >> +++ b/drivers/bus/mhi/host/boot.c >> @@ -584,6 +584,13 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) >> * device transitioning into MHI READY state >> */ >> if (fw_load_type == MHI_FW_LOAD_FBC) { >> + dev_dbg(dev, "standard_elf_image:%s\n", >> + (mhi_cntrl->standard_elf_image ? "True" : "False")); > > This print is just a noise even for debug. > >> + if (mhi_cntrl->standard_elf_image) { >> + fw_data += mhi_cntrl->sbl_size; >> + fw_sz -= mhi_cntrl->sbl_size; > > Is it possible to detect the image type during runtime instead of using a flag? > Also, the flag is currently unused. So it should come along an user. The flag would be used when a new WLAN device getting upstream. So either we merge this patch alone, or we get it grouped within the WLAN patches. Kindly share your thoughts? > > - Mani >
On Thu, Jun 05, 2025 at 10:34:50PM +0530, Manivannan Sadhasivam wrote: > On Tue, Jun 03, 2025 at 02:05:44AM -0700, Qiang Yu wrote: > > From: Mayank Rana <mayank.rana@oss.qualcomm.com> > > > > Currently, the FBC image is a non-standard ELF file that contains a single > > ELF header, followed by segments for SBL, RDDM, and AMSS. Some devices are > > unable to process this non-standard ELF format and therefore require > > special handling during image loading. > > > > What are those "some devices"? Why are they not able to process this format Eg. QCC2072 > which is used across the rest of the Qcom devices? These devices include TME-L (Trust Management Engine Lite). Currently, the FBC image is a non-standard ELF file containing an ELF header followed by segments for SBL and WLAN firmware. The ELF header and SBL segment within the first 512KB are loaded via BHI, while the full FBC image is loaded via BHIe. Due to TME-L limitations, the full FBC image loaded via BHIe cannot be processed, as it does not conform to the standard ELF format. > > > Add standard_elf_image flag to determine whether the device can process > > the non-standard ELF format. If this flag is set, a standard ELF image > > must be loaded, meaning the first 512 KB of the FBC image should be > > skipped when loading the AMSS image over the BHIe interface. > > Please explain what is present in the first 512KiB and why skipping that is > required. ELF header and SBL segment are in the first 512KiB. New FBC image format adds second ELF header in the start of WLAN FW segment on top of current format. After loading SBL, second ELF header and WLAN FW segment is loaded using BHIe. > > > Note that > > this flag does not affect the SBL image download process. > > > > Signed-off-by: Mayank Rana <mayank.rana@oss.qualcomm.com> > > Co-developed-by: Qiang Yu <qiang.yu@oss.qualcomm.com> > > Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com> > > --- > > Changes in v2: > > - V1 patch is paused because of no user. WLAN team plan to add support for > > new WLAN chip that requires this patch, so send v2. > > - Change author and SOB with new mail address. > > - Reword commit message. > > - Place standard_elf_image flag after wake_set in struct mhi_controller > > - Link to v1: https://lore.kernel.org/mhi/1689907189-21844-1-git-send-email-quic_qianyu@quicinc.com/ > > --- > > drivers/bus/mhi/host/boot.c | 7 +++++++ > > include/linux/mhi.h | 4 ++++ > > 2 files changed, 11 insertions(+) > > > > diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c > > index efa3b6dddf4d2f937535243bd8e8ed32109150a4..f1686a8e0681d49f778838820b44f4c845ddbd1f 100644 > > --- a/drivers/bus/mhi/host/boot.c > > +++ b/drivers/bus/mhi/host/boot.c > > @@ -584,6 +584,13 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) > > * device transitioning into MHI READY state > > */ > > if (fw_load_type == MHI_FW_LOAD_FBC) { > > + dev_dbg(dev, "standard_elf_image:%s\n", > > + (mhi_cntrl->standard_elf_image ? "True" : "False")); > > This print is just a noise even for debug. Will drop it. > > > + if (mhi_cntrl->standard_elf_image) { > > + fw_data += mhi_cntrl->sbl_size; > > + fw_sz -= mhi_cntrl->sbl_size; > > Is it possible to detect the image type during runtime instead of using a flag? > Also, the flag is currently unused. So it should come along an user. Perhaps we can check the second ELF Magic Number, but I don't think it's safe to determine the format by doing such check. Using a flag is simple and safe. > > - Mani > > -- > மணிவண்ணன் சதாசிவம்
On Fri, Jun 06, 2025 at 09:58:06AM +0800, Baochen Qiang wrote: > > > On 6/6/2025 1:04 AM, Manivannan Sadhasivam wrote: > > On Tue, Jun 03, 2025 at 02:05:44AM -0700, Qiang Yu wrote: > >> From: Mayank Rana <mayank.rana@oss.qualcomm.com> > >> > >> Currently, the FBC image is a non-standard ELF file that contains a single > >> ELF header, followed by segments for SBL, RDDM, and AMSS. Some devices are > >> unable to process this non-standard ELF format and therefore require > >> special handling during image loading. > >> > > > > What are those "some devices"? Why are they not able to process this format > > which is used across the rest of the Qcom devices? > > > >> Add standard_elf_image flag to determine whether the device can process > >> the non-standard ELF format. If this flag is set, a standard ELF image > >> must be loaded, meaning the first 512 KB of the FBC image should be > >> skipped when loading the AMSS image over the BHIe interface. > > > > Please explain what is present in the first 512KiB and why skipping that is > > required. > > > >> Note that > >> this flag does not affect the SBL image download process. > >> > >> Signed-off-by: Mayank Rana <mayank.rana@oss.qualcomm.com> > >> Co-developed-by: Qiang Yu <qiang.yu@oss.qualcomm.com> > >> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com> > >> --- > >> Changes in v2: > >> - V1 patch is paused because of no user. WLAN team plan to add support for > >> new WLAN chip that requires this patch, so send v2. > >> - Change author and SOB with new mail address. > >> - Reword commit message. > >> - Place standard_elf_image flag after wake_set in struct mhi_controller > >> - Link to v1: https://lore.kernel.org/mhi/1689907189-21844-1-git-send-email-quic_qianyu@quicinc.com/ > >> --- > >> drivers/bus/mhi/host/boot.c | 7 +++++++ > >> include/linux/mhi.h | 4 ++++ > >> 2 files changed, 11 insertions(+) > >> > >> diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c > >> index efa3b6dddf4d2f937535243bd8e8ed32109150a4..f1686a8e0681d49f778838820b44f4c845ddbd1f 100644 > >> --- a/drivers/bus/mhi/host/boot.c > >> +++ b/drivers/bus/mhi/host/boot.c > >> @@ -584,6 +584,13 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) > >> * device transitioning into MHI READY state > >> */ > >> if (fw_load_type == MHI_FW_LOAD_FBC) { > >> + dev_dbg(dev, "standard_elf_image:%s\n", > >> + (mhi_cntrl->standard_elf_image ? "True" : "False")); > > > > This print is just a noise even for debug. > > > >> + if (mhi_cntrl->standard_elf_image) { > >> + fw_data += mhi_cntrl->sbl_size; > >> + fw_sz -= mhi_cntrl->sbl_size; > > > > Is it possible to detect the image type during runtime instead of using a flag? > > Also, the flag is currently unused. So it should come along an user. > > The flag would be used when a new WLAN device getting upstream. So either we merge this > patch alone, or we get it grouped within the WLAN patches. Kindly share your thoughts? > For the reason I mentioned in my previous reply, I don't think we should rely on the flag unless the WLAN device is shipped with *only* the new FW. If that is the case, then please send this patch when the ath driver support shows up. I do not want to merge a patch with an unused interface. - Mani
diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c index efa3b6dddf4d2f937535243bd8e8ed32109150a4..f1686a8e0681d49f778838820b44f4c845ddbd1f 100644 --- a/drivers/bus/mhi/host/boot.c +++ b/drivers/bus/mhi/host/boot.c @@ -584,6 +584,13 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) * device transitioning into MHI READY state */ if (fw_load_type == MHI_FW_LOAD_FBC) { + dev_dbg(dev, "standard_elf_image:%s\n", + (mhi_cntrl->standard_elf_image ? "True" : "False")); + if (mhi_cntrl->standard_elf_image) { + fw_data += mhi_cntrl->sbl_size; + fw_sz -= mhi_cntrl->sbl_size; + } + ret = mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->fbc_image, fw_sz); if (ret) { release_firmware(firmware); diff --git a/include/linux/mhi.h b/include/linux/mhi.h index dd372b0123a6da5107b807ff8fe940c567eb2030..48f2b50038519111aed2075f01d0704adfc8989e 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -360,6 +360,9 @@ struct mhi_controller_config { * @bounce_buf: Use of bounce buffer * @fbc_download: MHI host needs to do complete image transfer (optional) * @wake_set: Device wakeup set flag + * @standard_elf_image: Flag to determine whether the first 512 KB of the FBC + * image need to be skipped when loading AMSS image over + * BHIe interface (optional) * @irq_flags: irq flags passed to request_irq (optional) * @mru: the default MRU for the MHI device * @@ -445,6 +448,7 @@ struct mhi_controller { bool bounce_buf; bool fbc_download; bool wake_set; + bool standard_elf_image; unsigned long irq_flags; u32 mru; };